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
Do not cache non-existent JS config files forever #12211
Conversation
Missed a spot in babel#11906. Currently if you don't have a JS config file and then add one to your project, it will not be picked up without restarting the process. Changing from caching forever to never caching when files don't exist fixes the issue.
Build successful! You can test your changes in the REPL here: https://babeljs.io/repl/build/30479/ |
This pull request is automatically built and testable in CodeSandbox. To see build info of the built libraries, click here or the icon next to each commit SHA. Latest deployment of this branch, based on commit b2382aa:
|
Well, invalidating JS configs when they change is going to be pretty difficult anyway due to node's require cache. Not sure if it's totally possible. We'd need to track all of the dependencies loading by a config and also delete them from the cache. So if you want to close this that's ok. Might be nice to at least pick up when configs are added I guess. |
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.
This is an "easy win".
In general, properly invalidating a JS config file when it changes is probably impossible: if someone really needs that feature to be 100% reliable, they can use a JSON config file.
@nicolo-ribaudo @JLHwung We now have dev dependency tracking in Parcel (see parcel-bundler/parcel#5802) and can invalidate JS configs, babel plugins, and babel itself from node's require cache when they or any dependency change. This is now the only issue remaining. #11906 changed this for non-JS configs, so I'm not sure why JS configs should behave differently. In general caching when a file doesn't exist is not safe unless you also invalidate when that file is created. It would be great to get this merged if possible. Alternatively, if there were a publicly exposed API to invalidate babel's various config caches, that would also be acceptable. For example, at the moment we have to remove all of babel core from node's require cache when a plugin is updated due to another babel cache of config items. Ideally we wouldn't need to do this and instead we could just invalidate the individual plugin. |
@JLHwung Do we have actual perf numbers (for #12211 (comment))? If performance is a problem, we could slightly improve this by caching all the config files in a folder as a block, so that if the existing file is not deleted we don't need to check if there are new files (since that would be an error anyway). |
It's also safe to cache during a single build as long as it invalidates for subsequent builds in watch mode. That would improve perf as well. Parcel has all this info but we'd need an API to tell Babel about it. |
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.
Agreed. We don't have specific perf feedback after #11906 is shipped, assuming the extra IO is acceptable.
Missed a spot in #11906. Currently if you don't have a JS config file and then add one to your project, it will not be picked up without restarting the process. Changing from caching forever to never caching when files don't exist fixes the issue.
Fixes #1, Fixes #2