Make watchStorySpecifiers use @parcel/watcher #27081
Conversation
There was a problem hiding this comment.
One possibly deal breaking thing I noticed is that the watcher.subscribe call fails (well, rejects its promise) on non-existent directories. I see two paths to fixing this:
- Just don't subscribe to non-existent directories. This is simple, but not great UX, as it requires re-starting storybook if the directory is added. I know people using storybook have all kinds of wild workflows, so my fear is someone rm -rfs and autogenerates their story directory and watching just doesn't work for them.
- Watch the first existing parent directory (and possibly use ignore config to skip everything but the watched directory). This watches more files, but probably fixes the majority of cases.
Watchpack handles this by basically recursively watching directories down to the root. We couldn't really do that with this lib.
| ); | ||
| const subscriptions = await Promise.all(directories.map(watchDirectory)); | ||
| return () => { | ||
| return Promise.all(subscriptions.map((s) => s.unsubscribe())).then(() => {}); |
There was a problem hiding this comment.
Final then is just so we get Promise<void> instead of Promise<Array<void>>.
| // TODO: Double check these ignore rules work as expected. | ||
| ignore: ['**/.git', '**/node_modules'], |
There was a problem hiding this comment.
Options described here, so pretty sure this should work and we can remove this comment.
ignore - an array of paths or glob patterns to ignore. uses is-glob to distinguish paths from globs. glob patterns are parsed with micromatch (see features).
- paths can be relative or absolute and can either be files or directories. No events will be emitted about these files or directories or their children.
- glob patterns match on relative paths from the root that is watched. No events will be emitted for matching paths.
| (err) => { | ||
| console.log(err); | ||
| } |
There was a problem hiding this comment.
Not sure about the best way to handle errors coming from the watcher.
|
Thanks @heyimalex ! I plan to evaluate this PR and the other one later this week. 🙏 |
|
@shilman, can I help you somehow by bringing this over the finish line? |
|
@valentinpalkovic Thanks for offering to help out here. I discussed with @tmeasday and we are going to merge #27016 for now. If you want to take over this PR, that would be fantastic. We are worried about destabilizing Storybook so I see this as a longer term fix, but if you can get confident about the change, then we can figure out when is the right time to merge. |
Alternative to #27016 that replaces watchpack with @parcel/watcher. Have not tested, and probably won't have time to continue work.
Thread on vite repo about them adopting @parcel/watcher, just for other discussion on positives/negatives.