-
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
chore(netlify-cms): update for v2, add standalone build system #6356
Conversation
Deploy preview for gatsbygram ready! Built with commit 23a4816 |
Deploy preview for using-drupal ready! Built with commit 23a4816 |
Not sure why those failures are happening, not seeing them locally. |
Nice work, that's a much simpler Those test failures seem odd - I can replicate them by checking out this branch and doing the following:
I'll do some more digging to see if I can find what's causing the failures. |
That's fair, I'll switch it to webpack. |
I've created PR #6391 |
982176e
to
15ccb9e
Compare
@emanueleperuffo I mentioned this on your PR, but THANK YOU so much for digging into this. Our efforts just happened to overlap as the Gatsby/Netlify CMS use case has been one of the "fires I've let burn" for a bit while tending to other things. I took the opportunity to finish the v2 update, and also to fix longstanding issues with using custom preview pane styles during Gatsby local development. There are a lot of known issues with this plugin, especially as documented in #4516, and I think this PR addresses all known issues while keeping the plugin minimally complex. @m-allanson all finished here. |
3c88c93
to
c1fe8e1
Compare
The test and deploy failures are present in other pull requests too, I don't believe any of them are related to this PR. |
c1fe8e1
to
96f3be4
Compare
|
||
window.netlifyIdentity = netlifyIdentityWidget | ||
netlifyIdentityWidget.init() | ||
if (NETLIFY_CMS_PREVIEW_STYLES_SET) { |
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 CI builds are failing because this is an undefined variable.
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.
Ah, I see that now - it's not actually undefined, but failing linting. Weird that it passes locally.
96f3be4
to
fc398c3
Compare
@m-allanson @KyleAMathews I don't believe this PR is responsible for any test failures - can this be merged? |
fc398c3
to
fb3c181
Compare
fb3c181
to
2885a2a
Compare
Hmm can't recreate the test errors locally... @m-allanson or @pieh — any idea what those test errors mean? |
It turns out that React isn't included as a dependency anywhere in the repo. The @erquhart I got this working by adding |
Just wanted to add a big 🎉 for this :) What is the relation between this PR and #6391? Should just this one be merged? |
Looking forward to seeing this PR merged, this is currently blocking us from upgrading our site to Gatsby V2. Thanks for the awesome work @erquhart 🎉😎 |
Ahhhh! That makes sense @m-allanson! Weird though you can't push to his repo as I can 🤔 Anyways, I'll make the change and push it right now. |
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.
Yeah, tests are passing. Thanks @erquhart for the nice PR!
Published PR to gatsby-plugin-netlify-cms@2.0.0-beta.6 |
@m-allanson @KyleAMathews good catch on the dependency issue, thank you both so much for digging in. |
Previously we had this line
Is there any reason why now it's changed to reloading the page? |
It seems that this is no longer transpiling TypeScript. My preview templates are TypeScript, and previously this was fine because it used the webpack config from the main site, which includes the TypeScript plugin. Now they're failing to compile. Is there a way of exposing the Netlify Webpack config? |
@steverandy I was actually in the middle of troubleshooting why reloads weren't happening on login, that needs to be changed. @ascorbic the Netlify CMS webpack config uses any plugins that loaded before the Netlify CMS plugin itself. Try moving your Typescript plugin before the Netlify CMS plugin and see if that works. It would be worth considering how to capture all plugins used, regardless of placement, but the current setup should work for most. |
Is it possible to use version > 2.0.1 like 2.0.1-beta.6 so npm doesn't update the package to 2.0.1 which is for gatsby v1? |
@emanueleperuffo Hmmm, we should bump major version for v2 - we did that some time ago for packages in this repository, but it seems like we have missed this package |
Is anyone able to get this to generate the |
Fixes #4516, #6048, #6248.