Skip to content

Commit 47fa2fa

Browse files
kriszypclaude
andcommitted
fix(components): prevent eager EntryHandler creation before handleEntry() is called (RE-8)
Scope.#optionsWatcherChangeListener was creating an EntryHandler when a config change fired for the `files` key before handleEntry() had been called. The orphan handler's initial chokidar scan completed with no callback attached; subsequent handleEntry() calls reused the handler and missed all initial add events, causing jsResource to silently skip resources.js on the first hot-reload after being added to config.yaml. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
1 parent 88c94e6 commit 47fa2fa

2 files changed

Lines changed: 61 additions & 3 deletions

File tree

components/Scope.ts

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -271,10 +271,14 @@ export class Scope extends EventEmitter<ScopeEventsMap> {
271271
if (key[0] === 'files' || key[0] === 'urlPath') {
272272
// TODO: validate options
273273

274-
// If not entry handler exists then likely the config did not have `files` initially
275-
// Now, it does, so create a default entry handler.
274+
// If no entry handler exists yet, the plugin's handleApplication has not called
275+
// handleEntry() yet — or it hasn't been called at all for this scope. Either way,
276+
// when handleEntry() is eventually called it will read the current config via
277+
// getFilesOption() and create the handler with the correct, up-to-date values.
278+
// Eagerly creating an entry handler here would start chokidar's initial scan
279+
// before the plugin's callback is attached, causing the initial `add` events to
280+
// be missed (RE-8).
276281
if (!scope.#entryHandler) {
277-
scope.#entryHandler = scope.#createEntryHandler(config as FileAndURLPathConfig);
278282
return;
279283
}
280284

unitTests/components/Scope.test.js

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -335,6 +335,60 @@ describe('Scope', () => {
335335
await scope.close();
336336
});
337337

338+
it('should not create entry handler from options change before handleEntry is called (RE-8)', async () => {
339+
// Reproduce the race in RE-8: OptionsWatcher fires a `change` event for the
340+
// `files` key BEFORE handleApplication (and thus handleEntry) runs. In the
341+
// broken code, #optionsWatcherChangeListener would create an entry handler
342+
// without any plugin callback attached. Chokidar's initial scan would then
343+
// emit `add` events with no consumer. When handleEntry was later called it
344+
// reused the existing handler — but the initial `add` events were already gone.
345+
writeFileSync(this.configFilePath, stringify({ [this.pluginName]: { files: 'test.js' } }));
346+
347+
const scope = new Scope(
348+
this.appName,
349+
this.pluginName,
350+
this.directory,
351+
this.configFilePath,
352+
new ApplicationScope('test', this.resources, this.server)
353+
);
354+
355+
await scope.ready;
356+
357+
// Simulate the race: emit a `change` event on scope.options for the `files`
358+
// key, as if OptionsWatcher's second config read (triggered by chokidar's own
359+
// `ready` event) completed before handleApplication called handleEntry. In the
360+
// broken code this created an entry handler immediately, starting a chokidar
361+
// scan with no plugin callback attached.
362+
scope.options.emit('change', ['files'], 'test.js', { files: 'test.js' });
363+
364+
// Yield to the event loop long enough for any spuriously-created entry handler
365+
// to start its chokidar watcher and complete its initial scan. In the broken
366+
// code the scan would fire `add` events here with no listener; in the fixed
367+
// code no entry handler is created at all during the change event.
368+
await new Promise((resolve) => setTimeout(resolve, 100));
369+
370+
// Now call handleEntry — this is what jsResource.handleApplication does.
371+
// If the bug is present, the entry handler already exists (from the change
372+
// listener) and its initial scan has already fired and is gone. The callback
373+
// would never receive the initial `add` event and the test would time out.
374+
const handleEntrySpy = spy();
375+
const entryHandler = scope.handleEntry(handleEntrySpy);
376+
assert.ok(entryHandler instanceof EntryHandler, 'Entry handler should be created');
377+
378+
// The callback must receive the initial `add` event for test.js.
379+
// In the buggy code this would time out because initial scan events are lost.
380+
await waitFor(() => handleEntrySpy.callCount > 0, {
381+
timeout: 2000,
382+
message: 'handleEntry callback must be called with initial add event (RE-8 regression)',
383+
});
384+
385+
const firstCall = handleEntrySpy.getCall(0).args[0];
386+
assert.equal(firstCall.eventType, 'add', 'initial event should be `add`');
387+
assert.equal(firstCall.absolutePath, this.testFilePath, 'initial event should be for the test file');
388+
389+
await scope.close();
390+
});
391+
338392
describe('deploy lifecycle integration', () => {
339393
// These cases ensure that when a deploy is in flight for the parent
340394
// component, file changes from the deploy itself (extract + npm install)

0 commit comments

Comments
 (0)