Skip to content

[BUGFIX release] Re-add the --watcher flag to build, so that when using vite, we don't run out of watchers on macOS, and allow watchman usage#10859

Open
NullVoxPopuli wants to merge 2 commits intomasterfrom
nvp/re-add-the-watcher-flag
Open

[BUGFIX release] Re-add the --watcher flag to build, so that when using vite, we don't run out of watchers on macOS, and allow watchman usage#10859
NullVoxPopuli wants to merge 2 commits intomasterfrom
nvp/re-add-the-watcher-flag

Conversation

@NullVoxPopuli
Copy link
Contributor

@NullVoxPopuli NullVoxPopuli commented Oct 28, 2025

Exploration in Discord: https://discord.com/channels/480462759797063690/568935504288940056/1432791167799656469

The problem is here:

  • if (this.hasOption('watcher')) {

    We never enter this block unless a watcher option is present. when doing ember s, watcher has a default value of "events", specified here:
    { name: 'watcher', type: String, default: 'events', aliases: ['w'] },
    -- but in the build command (edited in this PR) -- we were omitting that in the availableOptions during $EMBROIDER_PREBUILD -- which opts the build --watch that compatPrebuild is doing out of using watchman (which is a requirement to do anything on macOS with watched files).

NOTE: availableOptions was found as being relevant while debugging parseArgs

An alternate implementation of this PR is to always use watchman if process.env.EMBROIDER_PREBUILD is present (or rather, assume we "have a watcher" -- and let our call to WatchDetector figure it out -- (Linux, for example, doesn't need watchman))

… run out of watchers on macOS, and allow watchman usage
@kategengler
Copy link
Member

anyway to add a test?

@NullVoxPopuli
Copy link
Contributor Author

other than fixing the fixture/output tests, there is 0 prior art to testing watchman in the repo, and we don't test on macOS, so ... testing the specific situation here might be more effort than it's worth.

Adding the --watcher option though is not needed if we just always assume it's available when EMBROIDER_PREBUILD is present.

I think I'm going to do that -- in which case, tests should start passing

…BUILD), add a condition to modals/command around where watchman _would_ get used when EMBROIDER_PREBUILD is active
@kategengler
Copy link
Member

I didn't think we needed a test for watchman, just that the correct option is there with and without vite. I believe there are tests I edited when I changed the availableOptions where that would make sense.

Also we do test on MacOS... the entire matrix, I believe.

@kategengler
Copy link
Member

For ember-cli, using release-plan, you should target #release branch

@NullVoxPopuli
Copy link
Contributor Author

#10860

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants