Skip to content

Commit c49fe5c

Browse files
committed
chore(alias): address review feedback\n\n- tests: close Rollup bundle in helper to avoid leaks/flakes\n- types: narrow MapToFunction to explicit callable form (avoid global Function)
1 parent 51f0c28 commit c49fe5c

File tree

2 files changed

+46
-30
lines changed

2 files changed

+46
-30
lines changed

packages/alias/src/index.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,8 @@ import path from 'node:path';
33
// Public types are exported directly from source so tsc can emit declarations
44
import type { Plugin, PluginHooks } from 'rollup';
55

6-
type MapToFunction<T> = T extends Function ? T : never;
6+
// Narrow to explicit callable form instead of the global `Function` type.
7+
type MapToFunction<T> = T extends (...args: any[]) => any ? T : never;
78

89
export type ResolverFunction = MapToFunction<PluginHooks['resolveId']>;
910

packages/alias/test/alias.test.ts

Lines changed: 44 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -25,37 +25,52 @@ function resolveWithRollup(
2525
if (!plugins.find((p) => p.name === 'alias')) {
2626
throw new Error('`plugins` should include the alias plugin.');
2727
}
28-
return new Promise<string[] | (string | null)[]>((resolve, reject) => {
29-
rollup({
30-
input: 'dummy-input',
31-
plugins: [
32-
{
33-
name: 'test-plugin',
34-
buildStart() {
35-
resolve(
36-
Promise.all(
37-
tests.map(({ source, importer, options }) =>
38-
// @ts-expect-error - this context is provided by Rollup
39-
this.resolve(source, importer, options).then((result: any) =>
40-
result ? result.id : null
41-
)
42-
)
28+
29+
// Coordinate between the Rollup build lifecycle and our test assertions.
30+
let settle!: (value: (string | null)[]) => void;
31+
let settleErr!: (reason?: unknown) => void;
32+
const resultsPromise = new Promise<(string | null)[]>((res, rej) => {
33+
settle = res;
34+
settleErr = rej;
35+
});
36+
37+
return rollup({
38+
input: 'dummy-input',
39+
plugins: [
40+
{
41+
name: 'test-plugin',
42+
buildStart() {
43+
// Resolve the requested ids using Rollup's resolver in the proper context.
44+
Promise.all(
45+
tests.map(({ source, importer, options }) =>
46+
// @ts-expect-error - this context is provided by Rollup
47+
this.resolve(source, importer, options).then((result: any) =>
48+
result ? result.id : null
4349
)
44-
);
45-
},
46-
resolveId(id: string) {
47-
if (id === 'dummy-input') return id;
48-
if (externalIds.includes(id)) return { id, external: true } as any;
49-
return null;
50-
},
51-
load(id: string) {
52-
if (id === 'dummy-input') return 'console.log("test");';
53-
return null;
54-
}
50+
)
51+
)
52+
.then(settle)
53+
.catch(settleErr);
5554
},
56-
...plugins
57-
]
58-
}).catch(reject);
55+
resolveId(id: string) {
56+
if (id === 'dummy-input') return id;
57+
if (externalIds.includes(id)) return { id, external: true } as any;
58+
return null;
59+
},
60+
load(id: string) {
61+
if (id === 'dummy-input') return 'console.log("test");';
62+
return null;
63+
}
64+
},
65+
...plugins
66+
]
67+
}).then(async (bundle) => {
68+
try {
69+
const results = await resultsPromise;
70+
return results;
71+
} finally {
72+
await bundle.close();
73+
}
5974
});
6075
}
6176

0 commit comments

Comments
 (0)