embroider-build / embroider

Compiling Ember apps into spec-compliant, modern Javascript.
MIT License
339 stars 137 forks source link

v1 addon build watching gets stuck in a loop #1770

Open bendemboski opened 9 months ago

bendemboski commented 9 months ago

REPRO:

$ npx ember-cli@5.5 addon my-addon
$ cd my-addon
$ ember try:one embroider-optimized --- ember test -s

EXPECTED: The build runs once, and only rebuilds when something in the addon is modified

ACTUAL: The tests run, but there are continuous rebuilds in the background as the build watcher notices various changes in node_modules/.embroider

I reproduced this on MacOS with watchman installed, but I believe it will reproduce on any OS.

mansona commented 9 months ago

Does it also happen if you ran ember serve and went to localhost:4200/tests/ ?

bendemboski commented 9 months ago

@mansona no it doesn't

mansona commented 9 months ago

Well that is fishy 🤔 Is there any specific reason why you would want to use the ember test -s version over ember serve? I'm not saying that we shouldn't fix it or anything, I'm just trying to find out use cases here so I can understand the depth of the problem

bendemboski commented 9 months ago

Oh, a number of reasons:

bendemboski commented 5 months ago

Okay, I think I've finally gotten to the bottom of this and it's messy. I haven't had the chance to try this with watchers other than watchman, but I believe the root cause is that watchman by default ignores . directories, but only at the root of watched folders, e.g. if you watch /foo, it will ignore /foo/.something, but not /foo/bar/.something. So by default it does not ignore node_modules/.embroider.

The reason that this only reproduces with ember test -s and not ember serve is a happy accident -- ember serve adds an ignore path, while ember test -s doesn't, and only when any ignore paths are specified does this logic in sane activate and do its own checking, which apparently ignores any dot files/folders whether at the root or not. This is certainly a bug in sane, as the ignoring of dot files not at the root is determined by whether any ignore paths are present or not (probably the sane authors thought that watchman's dot file ignoring was not only for files at the root).

I think the solution here should be for ember-cli and/or embroider to not rely on the watcher ignoring the node_modules/.embroider directory, but to explicitly ensure that it is ignored. The most straightforward way I can see to do that is to add

ignored: [
  path.resolve(this.project.root, 'node_modules/.embroider'),
],

here and add

path.resolve(this.project.root, 'node_modules/.embroider'),

here (even though that's not strictly necessary to fix the bug). But that's kinda putting an embroider implementation detail in ember-cli...

I tried changing this to be a broccoli-funnel that excluded node_modules/.embroider, but that caused a severe build performance degradation.

So I'm not quite sure how to go about fixing this, and would love any thoughts.