Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for setting a force redirect status in Netlify _redirects file #8521

Conversation

RobertWSaunders
Copy link
Contributor

Proposed Change

Builds upon: #2890

When adding multiple domains in Netlify it can be nice to redirect domain aliases to your primary domain. In order to do so you need to add the correct redirect information into a _redirects file in the root of your site folder. The gatsby-netlify-plugin provides a nice way to automatically generate this file for us using the createRedirect action, see the documentation for it here. However, it does not allow us to set a redirect status with the force option which is mentioned in the Netlify redirect documentation here. In order for domain aliases redirects and any other redirect that wants to override existing content in the path to work correctly we need to add the ability to set a forced redirect status in our _redirects file. A forced status is denoted with an exclamation mark at the end of the status number. This change adds support for force: true to be passed into createRedirect which will result in a redirect status code with an exclamation mark concatenated to it in the outputted _redirects file.

For example:

createRedirect({ fromPath: "https://qhacks.ca/*", toPath: "https://qhacks.io/:splat", isPermanent: true, force: true });

Will output:

https://qhacks.ca/*  https://qhacks.io/:splat  301!

How did you do this?

Accept the force option to createRedirect and use it to build a redirect status.

Why are you choosing this approach?

Improves functionality of gatsby-netlify-plugin!

Any open questions you want to ask code reviewers?

Nope! 😎

List of changes:

  • Update createRedirect redux action!
  • Accept new force argument to createRedirect and build a redirect status from it.

@RobertWSaunders RobertWSaunders requested a review from a team as a code owner September 25, 2018 03:42
@RobertWSaunders
Copy link
Contributor Author

cc @bvaughn @KyleAMathews

@pieh
Copy link
Contributor

pieh commented Sep 25, 2018

I think this is fine, I just don't like adjusting gatsby API docs for netlify specific redirect flag - this probably belong to netlify plugin

@RobertWSaunders
Copy link
Contributor Author

So @pieh instead of adding to Gatsby createRedirect action docs, document this in the gatsby-netlify-plugin? I can add the appropriate docs, would the README for the plugin be a good place?

@pieh
Copy link
Contributor

pieh commented Sep 25, 2018

Yeah, that would be great @RobertWSaunders!

@RobertWSaunders RobertWSaunders force-pushed the topics/add-force-option-to-netlify-redirects branch from e928cbd to 6bc3267 Compare September 26, 2018 19:44
@RobertWSaunders
Copy link
Contributor Author

Updated!

@shannonbux
Copy link
Contributor

shannonbux commented Sep 27, 2018

Thanks for filling out the PR so completely @RobertWSaunders! @pieh I'll let you merge this one just in case there's any more technical feedback

@RobertWSaunders
Copy link
Contributor Author

Any updates on this @pieh?

Copy link
Contributor

@pieh pieh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks @RobertWSaunders and sorry for wait

@pieh pieh merged commit e1d354e into gatsbyjs:master Oct 2, 2018
@gatsbot
Copy link

gatsbot bot commented Oct 2, 2018

Holy buckets, @RobertWSaunders — we just merged your PR to Gatsby! 💪💜

Gatsby is built by awesome people like you. Let us say “thanks” in two ways:

  1. We’d like to send you some Gatsby swag. As a token of our appreciation, you can go to the Gatsby Swag Store and log in with your GitHub account to get a coupon code good for one free piece of swag. (Currently we’ve got a couple t-shirts available, plus some socks that are really razzing our berries right now.)
  2. We just invited you to join the Gatsby organization on GitHub. This will add you to our team of maintainers. You’ll receive an email shortly asking you to confirm. By joining the team, you’ll be able to label issues, review pull requests, and merge approved pull requests.

If there’s anything we can do to help, please don’t hesitate to reach out to us: tweet at @gatsbyjs and we’ll come a-runnin’.

Thanks again!

@RobertWSaunders RobertWSaunders deleted the topics/add-force-option-to-netlify-redirects branch October 2, 2018 20:56
@RobertWSaunders
Copy link
Contributor Author

Awesome, thanks @pieh!

lipis added a commit to lipis/gatsby that referenced this pull request Oct 3, 2018
* 'master' of github.com:gatsbyjs/gatsby: (72 commits)
  fix: fix wording in plopfile (gatsbyjs#8735)
  fix: navigateTo deprecation message (gatsbyjs#8745)
  Add deploying-to-heroku.md (gatsbyjs#8721)
  fix(docs): Link to instructions on adding to plugin library (gatsbyjs#8686)
  updated gatsby-plugin-remove-trailing-slashes docs to include link re… (gatsbyjs#8720)
  feat(www/starters): Add TypeScript and Contentful starter (gatsbyjs#8704)
  fix(docs): fix typos in template doc (gatsbyjs#8692)
  feat(www): change chevron direction on sticky responsive sidebar (gatsbyjs#8664)
  chore(release): Publish
  [integration] [cypress-gatsby] feat: Add Gatsby API support to our Cypress plugin (gatsbyjs#8641)
  chore(release): Publish
  fix(gatsby): add mjs config to webpack and resolve correctly (gatsbyjs#8717)
  feat(gatsby-plugin-netlify): add force option to createRedirect (gatsbyjs#8521)
  [www/starters] tweaks to fix DX for no GH token on www (gatsbyjs#8718)
  chore(release): Publish
  fix(gatsby-dev-cli): infer correct prefix from package path (gatsbyjs#8683)
  fix(www/starters): fix broken link to submission instructions (gatsbyjs#8715)
  chore(release): Publish
  feat(gatsby) : add createContentDigest helper (gatsbyjs#8687)
  feat: publish SendGrid case study blogpost (gatsbyjs#8592)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants