-
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
fix(gatsby-plugin-netlify-cms): Mark relevant dependencies as externals #19575
fix(gatsby-plugin-netlify-cms): Mark relevant dependencies as externals #19575
Conversation
cc3a3c9
to
ec5424b
Compare
ec5424b
to
1459b05
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.
@LekoArts this is good to go 👍
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.
Thank you so much! 🥇
Holy buckets, @erezrokah — we just merged your PR to Gatsby! 💪💜 Gatsby is built by awesome people like you. Let us say “thanks” in two ways:
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! |
Fixes #18245
Fixes #17568
I can split this into two PRs (one per issue) if needed.
Main changes:
Mark
netlify-cms-app
and relevant dependencies as externals to avoid re-build during bundle.netlify-cms-app
,netlify-identity-widget
,react
andreact-dom
are already set up for that since they expose a global variable on the window object which Webpack can reference instead of importing the module.The reason
netlify-identity-widget
was still included in the main app bundle here is that it didn't pass Gatsby's default thresholds for code splitting. Forcing it into its own cache group makes sure it's not in the app bundle.It will still be included in the html as a link tag with a preload attribute, since it is imported as a runtime dependency (though we do know at build time it won't be needed). I actually wrote some code to remove it from the html using agatsby-ssr.js
but then realized it will prevent using the widget all together in other contexts than the plugin (not sure such a scenario exists).Along with code splitting the widget when
enableIdentityWidget
is true, when the widget is disabled it will be ignored completely from the build via thewebpack.IgnorePlugin
plugin.Would love to get some feedback and if there is something I can do to make it easier for other people to test it please let me know (thought about publishing it separately).
Also, I'm not sure how to go about versioning.
Update:
Also fixes #15126