Improve watch mode

[faergeek]

Hello! Really cool project!

But I think the watch mode needs some improvements. Right now it watches all js files in the current directory, excluding node_modules. So now it doesn't work for css, for example. And I think you can imagine how many unnecessary reruns there can be. So I made it watch all the files from config.checks. Not sure if using flatMap is ok for supported node versions though. I can replace it if needed.

Also, despite package.json being watched, changes there are not applied, so I removed it for now to avoid confusion. I'm going to make it apply package.json changes as a next step and I have some more improvements in my mind related to watch mode.

I'm just not sure if it's ok to do in a single PR or multiple. What do you think?

[faergeek]

I'm also not sure how to test it properly yet. But I think existing tests for watch mode are not enough. And these tests actually make you add --forceExit so I would remove them and add tests for bin.js instead, because we need to kill process, which can't be easily done in run.js tests where process is being mocked

[ai]

This fix will not work for require('./other-file.js').

We can replace '**/*.js' to `'*/' (or maybe just ignore pattern).

[faergeek]

Is that a case for a library? Meaning it's needed only in cases where webpack is also being used?

[ai]

Meaning it's needed only in cases where webpack is also being used?

Yeap. require('') case is for @size-limit/webpack plugin cases. But it is a popular use case.

[faergeek]

It would be convenient to use webpack's watch mode in such cases then. Or we could allow a plugin to tell which files to add to watch or something like that. But that complicates things a bit. I'll try to think of some simpler solution. Would it be too bad to extend the interface between a tool and a plugin, if needed?

[ai]

Would it be too bad to extend the interface between a tool and a plugin, if needed?

We can do it, but it looks like overkill. We do not need 100% accurate method. It is OK to call Size Limit again more often that it was needed.

I think it is better to remove .js from watch pattern.

[faergeek]

What if we use the same info for watching, which file plugin uses for measurement? Like that https://github.com/ai/size-limit/blob/08f8af44769d8ff5e9346bb6cdd2d8c28f344299/packages/file/index.js#L50. I think it makes sense to setup watching after first run is complete. And we'll probably need to stop watching before running the next step, because ideally we would also reread package.json on every rerun (now it isn't reread every time), because the config can be changed completely. And files and plugins being used could also change and package.json could have syntax errors, etc.

So I suggest to run it like this:

1. Look at version and help cli flags. No change here.
2. Read package.json, validate config, setup plugins, create config and run plugins, all the usual stuff. No change here as well.
3. Get info about files from config the same way the file plugin does right now.
4. Setup watcher using info about files from plugins, plus also watch package.json for configuration changes
5. Once change is detected and we decide to rerun, we close the watcher and return to step 2.

If setting up watcher from scratch every time would be too expensive, chokidar has an API to add or remove files being watched.

[ai]

Looks very complicated and add false negative error to webpack use case.

Why you do not want to just remove .js from watch pattern?

[faergeek]

Removing .js solves only the problem of css not being watched. Using files returned from config doesn't change much actually. It's just like the change right now, but taking into account bundled key from config.

Other steps in the comment above are for reapplying changes from package.json. That's questionable and can/should be implemented separately then, I think.

[faergeek]

I think it would make sense to either reread package.json on changes or not to watch it at all

[ai]

Removing .js solves only the problem of css not being watched

Let’s solve one issues step-by-step. Let’s fix .css files watching by removing .js from watch pattern and then we will think about applying package.json config changes in watch mode.

[faergeek]

I pushed changes with only extension removal

[faergeek]

Should I then submit a separate PR with reloading package.json changes?

[ai]

Should I then submit a separate PR with reloading package.json changes?

Let's do it in a separate PR