Skip to content

Commit 3686a06

Browse files
gkalpakgkalpak
committed
fixup! fix(ngcc): cope with packages following APF v14+
1 parent 1e4527e commit 3686a06

File tree

3 files changed

+93
-13
lines changed

3 files changed

+93
-13
lines changed

‎packages/compiler-cli/ngcc/src/dependencies/module_resolver.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -137,7 +137,7 @@ export class ModuleResolver {
137137
*
138138
* This is achieved by checking for the existence of `${modulePath}/package.json`.
139139
* If there is no `package.json`, we check whether this is an APF v14+ secondary entry-point,
140-
* which does not have each own `package.json` but has an `exports` entry in the package's primary
140+
* which does not have its own `package.json` but has an `exports` entry in the package's primary
141141
* `package.json`.
142142
*/
143143
private isEntryPoint(modulePath: AbsoluteFsPath): boolean {

‎packages/compiler-cli/ngcc/src/utils.ts‎

Lines changed: 31 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -201,18 +201,42 @@ export function loadJson<T extends JsonObject = JsonObject>(
201201
export function loadSecondaryEntryPointInfoForApfV14(
202202
fs: ReadonlyFileSystem, primaryPackageJson: JsonObject|null, packagePath: AbsoluteFsPath,
203203
entryPointPath: AbsoluteFsPath): JsonObject|null {
204-
// Check if primary `package.json` has been loaded and has an `exports` property.
205-
if (primaryPackageJson?.exports === undefined) {
204+
// Check if primary `package.json` has been loaded and has an `exports` property that is an
205+
// object.
206+
const exportMap = primaryPackageJson?.exports;
207+
if (!isExportObject(exportMap)) {
206208
return null;
207209
}
208210

209211
// Find the `exports` key for the secondary entry-point.
210212
const relativeEntryPointPath = fs.relative(packagePath, entryPointPath);
211-
const exportsKey = `./${relativeEntryPointPath}`;
213+
const entryPointExportKey = `./${relativeEntryPointPath}`;
212214

213-
// Read the data (if it exists).
214-
const exportsData =
215-
(primaryPackageJson.exports as JsonObject)[exportsKey] as JsonObject | undefined;
215+
// Read the info for the entry-point.
216+
const entryPointInfo = exportMap[entryPointExportKey];
216217

217-
return exportsData ?? null;
218+
// Check whether the entry-point info exists and is an export map.
219+
return isExportObject(entryPointInfo) ? entryPointInfo : null;
220+
}
221+
222+
/**
223+
* Check whether a value read from a JSON file is a Node.js export map (either the top-level one or
224+
* one for a subpath).
225+
*
226+
* In `package.json` files, the `exports` field can be of type `Object | string | string[]`, but APF
227+
* v14+ uses an object with subpath exports for each entry-point, which in turn are conditional
228+
* exports (see references below). This function verifies that a value read from the top-level
229+
* `exports` field or a subpath is of type `Object` (and not `string` or `string[]`).
230+
*
231+
* References:
232+
* - https://nodejs.org/api/packages.html#exports
233+
* - https://nodejs.org/api/packages.html#subpath-exports
234+
* - https://nodejs.org/api/packages.html#conditional-exports
235+
* - https://v14.angular.io/guide/angular-package-format#exports
236+
*
237+
* @param thing The value read from the JSON file
238+
* @returns True if the value is an `Object` (and not an `Array`).
239+
*/
240+
function isExportObject(thing: JsonValue): thing is JsonObject {
241+
return (typeof thing === 'object') && (thing !== null) && !Array.isArray(thing);
218242
}

‎packages/compiler-cli/ngcc/test/utils_spec.ts‎

Lines changed: 61 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -283,7 +283,7 @@ runInEachFileSystem(() => {
283283
beforeEach(() => fs = getFileSystem());
284284

285285
describe('loadSecondaryEntryPointInfoForApfV14()', () => {
286-
it('should return `null` of the primary `package.json` failed to be loaded', () => {
286+
it('should return `null` if the primary `package.json` failed to be loaded', () => {
287287
expect(loadSecondaryEntryPointInfoForApfV14(fs, null, _abs('/foo'), _abs('/foo/bar')))
288288
.toBe(null);
289289
});
@@ -300,12 +300,39 @@ runInEachFileSystem(() => {
300300
.toBe(null);
301301
});
302302

303+
it('should return `null` if the primary `package.json`\'s `exports` property is a string',
304+
() => {
305+
const primaryPackageJson = {
306+
name: 'some-package',
307+
exports: './index.js',
308+
};
309+
310+
expect(loadSecondaryEntryPointInfoForApfV14(
311+
fs, primaryPackageJson, _abs('/foo'), _abs('/foo/bar')))
312+
.toBe(null);
313+
});
314+
315+
it('should return `null` if the primary `package.json`\'s `exports` property is a string array',
316+
() => {
317+
const primaryPackageJson = {
318+
name: 'some-package',
319+
exports: [
320+
'./foo.js',
321+
'./bar.js',
322+
],
323+
};
324+
325+
expect(loadSecondaryEntryPointInfoForApfV14(
326+
fs, primaryPackageJson, _abs('/foo'), _abs('/foo/bar')))
327+
.toBe(null);
328+
});
329+
303330
it('should return `null` if there is no info for the specified entry-point', () => {
304331
const primaryPackageJson = {
305332
name: 'some-package',
306333
exports: {
307334
'./baz': {
308-
isBar: false,
335+
main: './baz/index.js',
309336
},
310337
},
311338
};
@@ -315,19 +342,48 @@ runInEachFileSystem(() => {
315342
.toBe(null);
316343
});
317344

318-
it('should return the entry-point info if it exists', () => {
345+
it('should return `null` if the entry-point info is a string', () => {
346+
const primaryPackageJson = {
347+
name: 'some-package',
348+
exports: {
349+
'./bar': './bar/index.js',
350+
},
351+
};
352+
353+
expect(loadSecondaryEntryPointInfoForApfV14(
354+
fs, primaryPackageJson, _abs('/foo'), _abs('/foo/bar')))
355+
.toBe(null);
356+
});
357+
358+
it('should return `null` if the entry-point info is a string array', () => {
359+
const primaryPackageJson = {
360+
name: 'some-package',
361+
exports: {
362+
'./bar': [
363+
'./bar/a.js',
364+
'./bar/b.js',
365+
],
366+
},
367+
};
368+
369+
expect(loadSecondaryEntryPointInfoForApfV14(
370+
fs, primaryPackageJson, _abs('/foo'), _abs('/foo/bar')))
371+
.toBe(null);
372+
});
373+
374+
it('should return the entry-point info if it exists and is an object', () => {
319375
const primaryPackageJson = {
320376
name: 'some-package',
321377
exports: {
322378
'./bar': {
323-
isBar: true,
379+
main: './bar/index.js',
324380
},
325381
},
326382
};
327383

328384
expect(loadSecondaryEntryPointInfoForApfV14(
329385
fs, primaryPackageJson, _abs('/foo'), _abs('/foo/bar')))
330-
.toEqual({isBar: true});
386+
.toEqual({main: './bar/index.js'});
331387
});
332388
});
333389
});

0 commit comments

Comments
 (0)