Skip to content
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

Force rebuild on package.json changes #336

Closed
wants to merge 1 commit into from
Closed

Conversation

gaearon
Copy link
Contributor

@gaearon gaearon commented Aug 2, 2016

Partially fixes #186. I’m open to other solutions that don’t involve watching the whole node_modules or writing custom plugins.

I think this is a step in the right direction because at least you don’t have to restart for the watcher to pick up the changes, you just have to npm install --save.

This won’t help pick up updates you make by changing package.json and then running npm install, but they didn’t work anyway (you currently have to restart).

If there is a cheap way to watch for any changes to node_modules and purge the cache, we should probably do that instead.

@ghost ghost added the CLA Signed label Aug 2, 2016
// we want to rebuild the app and disregard the cached "module not found".
// This custom plugin does the job.
// Relevant issue: https://github.com/facebookincubator/create-react-app/issues/186
function RebuildOnPackageJSONChangePlugin(packageJsonPath) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it make sense to declare the plugin in a separate file?

Copy link
Contributor Author

@gaearon gaearon Aug 2, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sort of, I wouldn't worry about this for now though. I don't want people to start depending on it or think it's a "best practice". Ideally we shouldn't need it because there should be a better way to fix this.

@timmfin
Copy link

timmfin commented Aug 3, 2016

Maybe it is too simple to fully work in practice, but you could have a postinstall run script that wrote a timestamp or hash to a file to someplace like node_modules/deps-hash.txt. And the have a webpack plugin that watches that file.

But that unfortunately requires the hook to always present in the package.json (which is easy to accidentally remove)... Though maybe the webpack plugin could automatically check if the script is present in postinstall and add it if missing?

@leecade
Copy link

leecade commented Aug 3, 2016

👍 interested

@gaearon
Copy link
Contributor Author

gaearon commented Aug 3, 2016

Okay, I have a better idea of what’s going on here.

When you import a missing file directly, Webpack watches that file. So if it appears, everything works.

When you import something like react-bootstrap which in turn gets resolved via main field in package.json, webpack doesn’t know what to watch so it has no idea the package appeared later.

@TheLarkInn
Copy link

TheLarkInn commented Aug 3, 2016

When you import something like react-bootstrap which in turn gets resolved via main field in package.json, webpack doesn’t know what to watch so it has no idea the package appeared later.

Yeah that makes a lot of sense. I guess an enhancement would be to add a few supporting "guesses" like "./src/index.js", "./src/main.js", "./src/", "./src/whateverisgenericenoughtoguess.js". If it adds to the experience (for the watching capabilities).

But we wouldn't want to many empty watchers Id imagine.

@gaearon
Copy link
Contributor Author

gaearon commented Aug 3, 2016

Is there any way to watch node_modules itself just for "top level directory created" events?

@gaearon gaearon closed this Aug 3, 2016
@gaearon gaearon deleted the missing-deps branch August 3, 2016 21:52
@gaearon gaearon restored the missing-deps branch August 3, 2016 21:52
@gaearon gaearon deleted the missing-deps branch August 3, 2016 22:51
@lock lock bot locked and limited conversation to collaborators Jan 22, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Changes to node_modules should trigger recompilation
5 participants