Add check for unique redirect URLs

[yangshun]

Fixes #1.

It will be faster to do the checks within the redirect registration but I think doing the check in a separate function is cleaner. There aren't too many edges (currently less than 200) so I think performance is not an issue yet.

The build does not fail when errors are thrown because Gatsby catches them I think. I'm not sure how to get around this except using process.exit() instead of throwing an error. @bvaughn Any thoughts?

The errors are currently printed like this. Comments welcome!

⠁ The following paths are being redirected to different permalinks:
  - docs/animation-ja-JP.html → docs/create-fragment.html, docs/animation.html
  - docs/index.html → docs/accessibility.html, docs/hello-world.html
error Plugin default-site-plugin returned an error


  Error: Found the following redirect_froms to be duplicated:
    - docs/animation-ja-JP.html
    - docs/index.html
  
  - gatsby-node.js:95 Object.exports.createPages
    /Users/yangshun/Developer/reactjs.org/gatsby-node.js:95:11
  
  
  - next_tick.js:188 process._tickCallback
    internal/process/next_tick.js:188:7

[reactjs-bot]

Deploy preview ready!

Built with commit ad9a5c7

https://deploy-preview-47--reactjs.netlify.com

[yangshun]

Might have to change this to process.exit(1) instead of throwing an error if we want the build to halt.

[bvaughn]

Agreed. Replacing the throw with process.exit(1) seems like the right thing to do.

[bvaughn]

Agreed. Replacing the throw with process.exit(1) seems like the right thing to do.

[bvaughn]

I think I see your point about the separate function being clearer but it feels a bit "meh" to add another for-each.

What are your thoughts about doing the check inside of createPages, right before we call createRedirect? We could still use this technique of redirect-to-slug-map and just fail the first time we saw a key that already existed. This would only warn about the first duplicate (where as your approach warns about all duplicates) but once this check is in place, there should never be duplicates to begin with, so presumably if we found one to warn about- it would be the only one.

For example:

diff --git a/gatsby-node.js b/gatsby-node.js
index ab60095a6..07d93da1b 100644
--- a/gatsby-node.js
+++ b/gatsby-node.js
@@ -28,6 +28,9 @@ exports.modifyWebpackConfig = ({config, stage}) => {
 exports.createPages = async ({graphql, boundActionCreators}) => {
   const {createPage, createRedirect} = boundActionCreators;
 
+  // Used to detect and prevent duplicate redirects
+  const redirectToSlugMap = {};
+
   const blogTemplate = resolve('./src/templates/blog.js');
   const communityTemplate = resolve('./src/templates/community.js');
   const docsTemplate = resolve('./src/templates/docs.js');
@@ -114,13 +117,23 @@ exports.createPages = async ({graphql, boundActionCreators}) => {
           redirect = [redirect];
         }
 
-        redirect.forEach(fromPath =>
+        redirect.forEach(fromPath => {
+          if (redirectToSlugMap[fromPath] != null) {
+            console.error(`Duplicate redirect detected from "${fromPath}" to:\n` +
+              `* ${redirectToSlugMap[fromPath]}\n` +
+              `* ${slug}\n`
+            );
+            process.exit(1);
+          }
+
+          redirectToSlugMap[fromPath] = slug;
+
           createRedirect({
             fromPath: `/${fromPath}`,
             redirectInBrowser: true,
             toPath: `/${slug}`,
-          }),
-        );
+          });
+        });
       }
     }
   });

[yangshun]

I get what you mean. I was afraid that it would complicate the createPages function too much. I'll change it!

[yangshun]

@bvaughn Thanks for your feedback and the idea. I literally changed the PR to the code you provided 😂

Now the errors look like this. Pretty clear IMO 👍

...
⠁ Duplicate redirect detected from "docs/index.html" to:
* docs/accessibility.html
* docs/hello-world.html

error Command failed with exit code 1.
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.

[bvaughn]

😆 well I'm glad that worked out then.

Thanks again!