-
Notifications
You must be signed in to change notification settings - Fork 140
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Watch previous detected files in publicEntrypoints plugin #2208
Conversation
23bf8f8
to
3652b36
Compare
The fix in public-entry points looks good. The changes to the test waiting don't make sense to me. We're doubling down on time-based testing here when we should have none. If every single The point of nextBuild is to observe rollup completing a full build. A delay before calling A delay after nextBuild would only be needed after rollup reports a build complete, we still can't observe its output on the filesystem. If we're sure that's happening, then we should build polling into |
If anything, the possible race condition I see here would be calling |
the time delays are more for preventing chokidar from throttling and merging events that it thinks are atomic. but maybe those are not needed anymore |
okay, so that could then be related to the 150ms delay I introduced. But there was a delay before |
i get failures when i try again with 10ms |
logging start / end events it shows that we have 2 builds when we expect one. I suspect the update to package.json is triggering it.
|
@ef4 this is ready for another look. I updated the description with my new findings. I think i have figured out the real issues. |
b65cea8
to
1f2f893
Compare
ci run with extra debug logs https://github.com/embroider-build/embroider/actions/runs/12349262324/job/34459871601#step:5:1 |
a0712c4
to
4a19c87
Compare
4a19c87
to
d0f9bc0
Compare
bug
in public entrypoints we only add watch files. not dirs. When a file is removed, rollup also stops watching that file and ignores it if its re-created.
We need to watch previous detected files.
Testing Bug
Package.json is updated during the process which triggers a new build, we need to wait
Other things:
. Need to call result.close https://rollupjs.org/javascript-api/#rollup-watch
Not sure about nodejs/node#4760