Add support for additional/custom Netlify redirect rule criteria

[bvaughn]

The Netlify redirects documentation has a section titled "GeoIP and Language-based redirects" that outlines how to specify country and language based redirect rules. This PR adds support for these by adding ...rest params to the redirect action and consuming them in the gatsby-plugin-netlify plug-in.

For example, given the following calls:

createRedirect({
  fromPath: '/index.html',
  redirectInBrowser: true,
  toPath: '/',
});
createRedirect({
  fromPath: '/index.html',
  toPath: '/ja/index.html',
  Language: 'ja',
});
createRedirect({
  fromPath: '/index.html',
  toPath: '/zh-CN/index.html',
  Language: 'zh',
  Country: 'CN',
});

The updated plug-in will now generate the following _redirects content:

/index.html  /  302
/index.html  /ja/index.html  302  Language=ja
/index.html  /zh-CN/index.html  302  Language=zh  Country=CN

The plug-in also warns about values with spaces in them (since at least the Netlify plug-in doesn't support them). For example:

createRedirect({
  fromPath: '/foo.html',
  toPath: '/bar.html',
  foo: 'bar baz'
});

Will log the following warning:

Invalid redirect value "bar baz" specified for key "foo". Values should not contain spaces.

Note that this PR will also add the ability to support additional Netlify redirect rules like "Role".

[gatsbybot]

Deploy preview ready!

Built with commit 23f1527

https://deploy-preview-2890--gatsbygram.netlify.com

[gatsbybot]

Deploy preview ready!

Built with commit 23f1527

https://deploy-preview-2890--using-drupal.netlify.com

[Daniel15]

I wonder if it's worth treating the key/value pairs as an array and joining them with ,:

  keyValuePairs: {
    Language: ['zh', 'ja'],
  },

Right now you'd need to know to combine them with a single comma with no space, which is a leaky abstraction:

  keyValuePairs: {
    Language: 'zh,ja',
  },

I think "settings" or "config" is a good name for them. "keyValuePairs" is pretty generic - It explains the data type rather than the purpose 😄

[Daniel15]

As a side note, for a future enhancement it would be good to support 200, which serves the file without redirecting. A use case is for a simple single-page app where you want all URLs to go to the one actual HTML page. Also, 404, which serves a custom 404 page for the URL (returns the provided HTML file with a 404 status code). Maybe instead of a boolean isRedirect, this could be a mode enum with 'temporaryRedirect', 'permanentRedirect', 'internalRedirect' (200) or 'fileNotFound'? Totally outside the scope of this PR though :)

[Daniel15]

Maybe call this settings?

[bvaughn]

I wonder if it's worth treating the key/value pairs as an array and joining them with
Right now you'd need to know to combine them with a single comma with no space, which is a leaky abstraction

Hm. Maybe. I think string-to-string mapping is nice.

The plug-in specifically warns if you provide a value with a space in it, FWIW. As for "leaky" - I think it's reasonable for users to read the documentation if they're writing redirect rules 😉

I think "settings" or "config" is a good name for them. "keyValuePairs" is pretty generic - It explains the data type rather than the purpose 😄

Naming is hard. 🤔 This could be used for lots of things - language, country, role, etc. - so I opted for a name that at least described the shape of the data. I think "settings" and "config" is even more vague than "keyValuePairs". Neither describes the purpose but at least the latter hints at the structure. I dunno. This is subjective. I don't feel super strongly about it.

I think of the whole data structure that's passed to the method as "settings" or "config" though.

[Daniel15]

Okay, I'm convinced (on both points) 😛

What about "extraData" instead of "keyValuePairs"?

oh man naming things is so hard

[bvaughn]

oh man naming things is so hard

Agreed! 😆

I guess I like "extraData" well enough. It's still pretty vague.

I found myself wanting to just use ...rest so we could just do:

createRedirect({
  fromPath: '/index.html',
  toPath: '/zh-CN/index.html',
  Language: 'zh',
  Role: 'admin',
});

But I'm not sure this is intuitive.

[Daniel15]

Oh yeah, that's pretty nice. The other option could be to pass two arguments to createRedirect?

createRedirect({
  fromPath: '/index.html',
  toPath: '/zh-CN/index.html',
}, {
  Language: 'zh',
  Role: 'admin',
});

That looks kinda ugly though.

[bvaughn]

TBH the main reason I didn't use ...rest initially was because I didn't know how to make it work with Kyle's parameter documentation syntax 😅 I think I like it the best of the options we've discussed above.

[bvaughn]

I think I'm going to change it to ...rest and just add an example into the docs.

Thanks for the discussion!

[Daniel15]

Sounds good! :D

[bvaughn]

Looks like "AWS CodeBuild" is failing but I don't have permissions to see why. 😄

[KyleAMathews]

Eh sorry about that — it's a WIP thing and doesn't mean anything right now. Rolling out soon though some super cool new integration testing :-) It'll be building/testing every example site on every commit + a bunch of community sites including reactjs.org!

[bvaughn]

That's really exciting 😁

[KyleAMathews]

For the repl tests & appveyor — I put up this PR #2908

We've faced this issue a few times already 🙄

[bvaughn]

Great! I'll back out that single commit then.

[KyleAMathews]

Thanks!

[bvaughn]

Thank you, as usual