Skip to content

Commit 43b13e4

Browse files
killaguclaude
andcommitted
fix(core): address PR review feedback
- Validate typescriptEnabled in manifest #validate() to reject stale TS/JS resolution caches when TypeScript mode changes - Fix globFiles() empty array bug: `if (cached)` treated [] as falsy - Fix triggerLoadMetadata() to propagate first error via this.ready(err) - Fix clean() to only ignore ENOENT, rethrow other errors - Use import.meta.url instead of __dirname in ESM test Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent 8219422 commit 43b13e4

3 files changed

Lines changed: 26 additions & 8 deletions

File tree

packages/core/src/lifecycle.ts

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -345,19 +345,22 @@ export class Lifecycle extends EventEmitter {
345345
async triggerLoadMetadata(): Promise<void> {
346346
this.#metadataOnly = true;
347347
debug('trigger loadMetadata start');
348+
let firstError: Error | undefined;
348349
for (const boot of this.#boots) {
349350
if (typeof boot.loadMetadata === 'function') {
350351
debug('trigger loadMetadata at %o', boot.fullPath);
351352
try {
352353
await boot.loadMetadata();
353354
} catch (err) {
354-
debug('trigger loadMetadata error at %o, error: %s', boot.fullPath, err);
355-
this.emit('error', err);
355+
const error = err instanceof Error ? err : new Error(String(err));
356+
if (!firstError) firstError = error;
357+
debug('trigger loadMetadata error at %o, error: %s', boot.fullPath, error);
358+
this.emit('error', error);
356359
}
357360
}
358361
}
359362
debug('trigger loadMetadata end');
360-
this.ready(true);
363+
this.ready(firstError ?? true);
361364
}
362365

363366
#initReady(): void {

packages/core/src/loader/manifest.ts

Lines changed: 17 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,8 @@ import fsp from 'node:fs/promises';
44
import path from 'node:path';
55
import { debuglog } from 'node:util';
66

7+
import { isSupportTypeScript } from '@eggjs/utils';
8+
79
const debug = debuglog('egg/core/loader/manifest');
810

911
const MANIFEST_VERSION = 1;
@@ -124,6 +126,16 @@ export class ManifestStore {
124126
return false;
125127
}
126128

129+
const currentTypescriptEnabled = isSupportTypeScript();
130+
if (inv.typescriptEnabled !== currentTypescriptEnabled) {
131+
debug(
132+
'manifest typescriptEnabled mismatch: expected %s, got %s',
133+
currentTypescriptEnabled,
134+
inv.typescriptEnabled,
135+
);
136+
return false;
137+
}
138+
127139
const currentLockfileFingerprint = ManifestStore.#lockfileFingerprint(baseDir);
128140
if (inv.lockfileFingerprint !== currentLockfileFingerprint) {
129141
debug('manifest lockfileFingerprint mismatch');
@@ -163,8 +175,9 @@ export class ManifestStore {
163175
*/
164176
globFiles(directory: string, fallback: () => string[]): string[] {
165177
const relKey = this.#toRelative(directory);
166-
const cached = this.data.fileDiscovery?.[relKey];
167-
if (cached) {
178+
const cache = this.data.fileDiscovery;
179+
if (cache && relKey in cache) {
180+
const cached = cache[relKey];
168181
debug('[globFiles:manifest] using cached files for %o, count: %d', directory, cached.length);
169182
return cached;
170183
}
@@ -216,8 +229,8 @@ export class ManifestStore {
216229
try {
217230
fs.unlinkSync(manifestPath);
218231
debug('manifest removed: %s', manifestPath);
219-
} catch {
220-
// file doesn't exist, nothing to do
232+
} catch (err: any) {
233+
if (err.code !== 'ENOENT') throw err;
221234
}
222235
}
223236

packages/core/test/loader/manifest_roundtrip.test.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import assert from 'node:assert/strict';
22
import fs from 'node:fs';
33
import path from 'node:path';
4+
import { fileURLToPath } from 'node:url';
45

56
import { describe, it, beforeEach, afterEach } from 'vitest';
67

@@ -99,7 +100,8 @@ describe('ManifestStore roundtrip: generate → write → load', () => {
99100
});
100101

101102
it('should match expected manifest fixture structure', () => {
102-
const fixturePath = path.join(__dirname, '../fixtures/manifest/expected-manifest.json');
103+
const currentDir = path.dirname(fileURLToPath(import.meta.url));
104+
const fixturePath = path.join(currentDir, '../fixtures/manifest/expected-manifest.json');
103105
const expected = JSON.parse(fs.readFileSync(fixturePath, 'utf-8'));
104106

105107
assert.equal(expected.version, 1);

0 commit comments

Comments
 (0)