-
Notifications
You must be signed in to change notification settings - Fork 4.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
Build: Ensure non-JS changes are processed in the watch mode #30343
Conversation
6a38132
to
b2206ff
Compare
This is really nice work, thank you. I can confirm that the CSS consistently compiles for me, when on this branch. Specifically I did first
and sure enough, red borders everywhere. Removing it again, gone. I'll let others sanity check the code, but thank you, this is quality of life 👌 |
Size Change: 0 B Total Size: 1.42 MB ℹ️ View Unchanged
|
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. Let's keep an eye on the upcoming days on whether the "watch" target change is impactful.
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.
Wait :P
@@ -270,7 +270,7 @@ module.exports = { | |||
new DependencyExtractionWebpackPlugin( { injectPolyfill: true } ), | |||
].filter( Boolean ), | |||
watchOptions: { | |||
ignored: '!packages/*/!(src)/**/*', | |||
ignored: '!packages/*/build-module/**/*', |
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.
Actually, this is going to watch everything expect the build-module folder which is the opposite of what we want it seems :P (the key is ignored
)
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.
It's a double negation, we tell ignore everything that isn't in the build-module
with !
character 🙃
The API for watchOptions
is very simple and doesn't provide the allowed list.
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 didn't see that !
there
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.
We will need RegExp to make it work with the PostCSS upgrade, it's even more fun:
ignored: /^(?!.*\/packages\/.*\/build-module\/.*)$/,
I'm including also a screencast to show that it works. There is a slight delay but I assume it's how long webpack processes the files: Screen.Recording.2021-03-29.at.14.39.24.mov |
What a rollercoaster! |
Description
Originally reported by @stokesman in #27821 (comment) after merging #27821 (it was reverted because of the issue reported).
A similar issue was previously reported and this PR should also fix #22645.
In addition to that, there were several reports that changes to SCSS files weren't propagate when using
npm run dev
(build in watch mode).Changes proposed
This PR upgrades the version of
CopyWebpackPlugin
to5.1.2
(there are newer versions that work only with webpack 5) and updates the list of ignored files in the watch mode to resolve both issues present.Now that
CopyWebpackPlugin
ensures we watch for changes for all files that it processes, we only need to look at changes applied to the packages inbuild-module
folder (all production packages are transpiled and have this folder).How has this been tested?
Run
npm run dev
and make sure that changes to files are reflected in the/build
folder. I tried changing the following files:packages/*/src/
folderblock.json
in theblock-library
packageTypes of changes
Bug fix (non-breaking change which fixes an issue).
Checklist:
*.native.js
files for terms that need renaming or removal).