-
Notifications
You must be signed in to change notification settings - Fork 10.3k
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
gatsby-plugin-netlify tweaks #2191
gatsby-plugin-netlify tweaks #2191
Conversation
* Only write '_redirects' file if redirects have been declared. * Append to '_redirects' file is user has defined one already in the 'static' folder. * Write header and redirects files in parallel. * Replaced 'pify' package with 'fs-extra'.
238819d
to
c21f259
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome, thanks for the changes
No problem! Thanks for kicking things off with this plugin 😁 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good!
|
||
return writeFile(publicFolder(`_redirects`), data.join(`\n`)) | ||
return fileExists |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The problem with this is that files in public aren't deleted between builds so each time you build the redirects file will just grow and grow.
It seems like a fairly rare thing that someone would both want to maintain a handwritten redirects file & have this plugin generating one. Perhaps make it a plugin option? E.g. appendAutogeneratedRedirectsToStaticRedirectsFile
haha or a shorter option name :-D
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh interesting. I didn't realize that b'c I've been manually clearing between builds.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My understanding is this: If a file exists in static/_redirects
it will be copied over each time the build command is run so it's safe to append. If no file exists, we'll create one initially and then (unless the user manually clears the contents of public
) that same file will be around the next time the build command is run.
If that's the case, we can just search for our "Created with gatsby-plugin-netlify" comment in order to determine if it's the file we created or not. It's a little hacky I guess, but I think it should work like:
- Is there a file?
- No: Write a new one.
- Yes: Does it contain the "Created with gatsby-plugin-netlify" comment?
- No: Then it's a user-generated file, copied from
static
; append to it. - Yes: Then it's a file we created previous; override it.
- No: Then it's a user-generated file, copied from
I'll push an update with this proposed change momentarily. I've tested it locally and it seems to work correctly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Love it! Complicated implementation > complicated UX. Seems like it should be pretty stable.
@@ -1,16 +1,25 @@ | |||
import fs from "fs" | |||
import pify from "pify" | |||
import { appendFile, exists, writeFile } from "fs-extra" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 to fs-extra. We use it everywhere in Gatsby.
Deploy preview ready! Built with commit 238819d71b2ce823f638359e083ee858576aa787 |
Deploy preview ready! Built with commit 9ba3cbb |
Don't override user redirects. Don't contiunously append to generated redirects either.
Okay 😄 Check out the most recent push. I think it should handle the cases we've discussed without requiring additional user configuration. |
Lovely! |
Thanks for being so responsive, Kyle! |
_redirects
file if redirects have been declared._redirects
file is user has defined one already in thestatic
folder.pify
package withfs-extra
.Follow-up to #2185