Optimize watched directories

[simonihmig]

With watchDependencies defined, eai is currently creating a WatchDir based on the package's root folder for handling imports of v2 addons.

The problem with this is that it will trigger a rebuild of the app when something changes outside of the folders where the actual importable files live in (which is in ./dist, assuming the conventional setup of v2 addons), most notably the ./src folder. This would make the app watcher fire multiple times in succession: once for the change in ./src, and later (when the rollup build has finished) for the change in ./dist. See also: https://github.com/embroider-build/embroider/pull/1901

Instead of watching the whole package folder, this PR is looking at the package.json exports (if existing) as the source of truth for what is importable, and derives a minimal set of folders containing these files, which are as close as possible to these files. For example, when an addon has ./dist/components/foo.js and ./dist/components/bar.js as the only files covered by exports, this would make ./dist/components be the only folder being watched, ignoring any changes in e.g. ./src or ./declarations.

Note that I am only referring to ./src or ./dist as examples, the code has none of these hard-coded (with one exception, see inline-comment), but as said above derives everything from the package.json. So the change here should work also in v2 setups not following our conventional setup based on the blueprint.

This is one puzzle piece of fixing https://github.com/embroider-build/addon-blueprint/issues/32. Again, I tested this in a local installation, and it did have the desired effect (when combined with the other PR): when changing only source files and not having the addon's build running in parallel, the app would not rebuild, as it would be expected.

[simonihmig]

The failing test is a weird one: there is an unexpected file (./mod/index.js,) in the scenario-tester project that fails the test. However, this file seems to be generated in a different test suite here, so seems something is leaking here. And indeed, when running the watch-utils tests in isolation, they do pass! Or maybe I am missing something stupid, maybe some scenario-tester expert is able to chime in here?

[ef4]

Discussed in tooling meeting, this seems fine and can move ahead when it has passing tests.

[simonihmig]

The failing test is a weird one: there is an unexpected file (./mod/index.js,) in the scenario-tester project that fails the test.

@mansona I think when we discussed this PR, you mentioned an issue about leaking stuff in scenario-tester that is being worked on. Has this been fixed meanwhile, or could you point me to an issue/PR I can track?

[mansona]

@simonihmig yes this would be included in this PR: https://github.com/embroider-build/scenario-tester/pull/34 which was released in scenario-tester@4 https://github.com/embroider-build/scenario-tester/pull/35

The underlying issue was in fixiturify project: https://github.com/stefanpenner/node-fixturify-project/pull/87

[simonihmig]

I see, thank you @mansona! So next step is getting us to 4.0.0 here, right?

This was only flagged a major because you could rely on the wrong behaviour that could break tests now, is that correct? Or any other things to be aware of?

[mansona]

I'm pretty sure that's the only notable change that is included in the major, there may be a few other type changes and one other API change but there is a 1-1 replacement if those changes are hit 👍 I think just trying it and working through should be good enough without too much other investigation

[simonihmig]

Tried to upgrade scenario-tester to v4 today, but getting a good amount of TS errors:

We are on a very outdated version of TS here (4.3.5), probably we have to get us to some more recent version first!? 😔

[simonihmig]

With updates to TS and scenario-tester merged and this one rebased, this is now finally having a green build! :tada:

Thus ready for a final review! cc @ef4 @mansona