Skip to content

Commit f63520b

Browse files
michaelfaithalan-agius4
authored andcommitted
fix(@angular-devkit/schematics): running external schematics with yarn pnp
This change addresses an issue encountered when running external schematics from a yarn pnp workspace. The function used to resolve a collection json using node used recursion in a way that it effectively walked itself into an exception. Then, if the exception is the type it expected, it would keep going. This was flawed in that yarn with pnp throws a different type of error when it failed to load the mis-constructed collection path (e.g. `/node_modules/@schematics/angular/collection.json/package.json`). `ENOTDIR` instead of `MODULE_NOT_FOUND`. This process of intentionally / knowingly walking into an exception seems problematic in general. So, I addressed it by first checking if the `schematics` entry in the package is a relative path. If it is, then don't construct the collection path from that. If entry is not relative, then assume it's pointing at another package and we need to recurse to get to the actual collection path. I've tested this in both yarn pnp and non-pnp environments.
1 parent 081111f commit f63520b

File tree

1 file changed

+16
-5
lines changed

1 file changed

+16
-5
lines changed

packages/angular_devkit/schematics/tools/node-module-engine-host.ts

Lines changed: 16 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,19 @@ export class NodeModulesEngineHost extends FileSystemEngineHostBase {
3333
super();
3434
}
3535

36-
private resolve(name: string, requester?: string): string {
36+
private resolve(name: string, requester?: string, references = new Set<string>()): string {
37+
// Keep track of the package requesting the schematic, in order to avoid infinite recursion
38+
if (requester) {
39+
if (references.has(requester)) {
40+
references.add(requester);
41+
throw new Error(
42+
'Circular schematic reference detected: ' + JSON.stringify(Array.from(references)),
43+
);
44+
} else {
45+
references.add(requester);
46+
}
47+
}
48+
3749
const relativeBase = requester ? dirname(requester) : process.cwd();
3850
let collectionPath: string | undefined = undefined;
3951

@@ -46,7 +58,6 @@ export class NodeModulesEngineHost extends FileSystemEngineHostBase {
4658
};
4759

4860
// Try to resolve as a package
49-
let possibleCollectionPath = name;
5061
try {
5162
const packageJsonPath = require.resolve(join(name, 'package.json'), resolveOptions);
5263
const { schematics } = require(packageJsonPath);
@@ -61,9 +72,9 @@ export class NodeModulesEngineHost extends FileSystemEngineHostBase {
6172
const packageDirectory = dirname(packageJsonPath);
6273
collectionPath = resolve(packageDirectory, schematics);
6374
}
64-
// Otherwise use the path as-is to attempt resolution
75+
// Otherwise treat this as a package, and recurse to find the collection path
6576
else {
66-
possibleCollectionPath = schematics;
77+
collectionPath = this.resolve(schematics, packageJsonPath, references);
6778
}
6879
} catch (e) {
6980
if ((e as NodeJS.ErrnoException).code !== 'MODULE_NOT_FOUND') {
@@ -74,7 +85,7 @@ export class NodeModulesEngineHost extends FileSystemEngineHostBase {
7485
// If not a package, try to resolve as a file
7586
if (!collectionPath) {
7687
try {
77-
collectionPath = require.resolve(possibleCollectionPath, resolveOptions);
88+
collectionPath = require.resolve(name, resolveOptions);
7889
} catch (e) {
7990
if ((e as NodeJS.ErrnoException).code !== 'MODULE_NOT_FOUND') {
8091
throw e;

0 commit comments

Comments
 (0)