import-js icon indicating copy to clipboard operation
import-js copied to clipboard

The daemon doesn't reindex dependencies after npm update.

Open dcporter opened this issue 8 years ago • 2 comments

I'm opening this as a splinter from #376, to track the problem where the daemon doesn't watch node_modules and therefore doesn't hear about it when you npm update (a reasonable thing to do).

dcporter avatar Feb 23 '17 17:02 dcporter

From #376:

I'll try to come up with a solution that doesn't involve having to watch node_modules for changes. If you have any ideas on that, let me know!

I have thought about this! It's an interesting design problem, I think the design is harder than the implementation but let's see.

Right okay so this is a tradeoffs question… watching things is expensive, so any new thing you watch has to be justified. Definitely npm update happens, and people do it with their editors open, so it should work. But watching every recursive directory for any changes in node_modules is presumably much too expensive. So what can we get away with watching to handle this use case?

  • Solid move only indexing exports from things in dependencies (+peer+[dev]). If my project imports stuff from a package, it had damn well better be listed in my dependencies. That narrows the focus right out of the gate.
  • I think the key insight is that you don't need to know if any file in a package changes, you just need to know if a file changes. That is, I believe npm update overwrites the directory entirely, so you should be able to watch just the directory (as an entity, not in terms of its contents). You could also just pick a file and watch that. (I know Watchman doesn't do this, so it'd have to be wired up with something else — a quick look at its docs suggests that the unpopular fs.watch might actually be sufficient here. There are others too.) Slightly special handling if the package is a symlink — you probably have to watch the both the symlink itself, in the case of npm unlink, and one file within it, in the case of git fetch or whatever. (The main file, returned from require.resolve, seems like a good candidate, except possibly see next item.)
  • Serious bonus points for dealing with the case that the package is under active development locally (the usual use case for npm link) — then maybe you have to watch the main file (the one returned from require.resolve) too. (And there's an issue with the common pattern of re-exporting * from the main file — which I'm going to file later — which may require that more files be watched, but that should be some pretty simple follow-the-tree recursion.) This one seems like a stretch goal to me (but a really super useful one), if you'd like I'd be happy to write it up as a separate issue?

So instead of watching everything in node_modules, which is a total non-starter, you should be able to get away with watching one thing per directly-depended-upon package. Some minor complications about which one thing to watch, but the main file from require.resolve should suffice, with the addition of the symlink itself in the case of symlink'd packages.

dcporter avatar Feb 23 '17 17:02 dcporter

This is a great summary! Thanks for taking the time, I think you're right in that we need to special-case package dependencies (and not use watchman here).

trotzig avatar Feb 23 '17 18:02 trotzig