Skip to content

Commit

Permalink
fix(@angular-devkit/schematics): running external schematics with yar…
Browse files Browse the repository at this point in the history
…n 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.

(cherry picked from commit f63520b)
  • Loading branch information
michaelfaith authored and alan-agius4 committed Oct 31, 2023
1 parent 5f4ca4e commit 2a1bbc0
Showing 1 changed file with 16 additions and 5 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,19 @@ export class NodeModulesEngineHost extends FileSystemEngineHostBase {
super();
}

private resolve(name: string, requester?: string): string {
private resolve(name: string, requester?: string, references = new Set<string>()): string {
// Keep track of the package requesting the schematic, in order to avoid infinite recursion
if (requester) {
if (references.has(requester)) {
references.add(requester);
throw new Error(
'Circular schematic reference detected: ' + JSON.stringify(Array.from(references)),
);
} else {
references.add(requester);
}
}

const relativeBase = requester ? dirname(requester) : process.cwd();
let collectionPath: string | undefined = undefined;

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

// Try to resolve as a package
let possibleCollectionPath = name;
try {
const packageJsonPath = require.resolve(join(name, 'package.json'), resolveOptions);
const { schematics } = require(packageJsonPath);
Expand All @@ -61,9 +72,9 @@ export class NodeModulesEngineHost extends FileSystemEngineHostBase {
const packageDirectory = dirname(packageJsonPath);
collectionPath = resolve(packageDirectory, schematics);
}
// Otherwise use the path as-is to attempt resolution
// Otherwise treat this as a package, and recurse to find the collection path
else {
possibleCollectionPath = schematics;
collectionPath = this.resolve(schematics, packageJsonPath, references);
}
} catch (e) {
if ((e as NodeJS.ErrnoException).code !== 'MODULE_NOT_FOUND') {
Expand All @@ -74,7 +85,7 @@ export class NodeModulesEngineHost extends FileSystemEngineHostBase {
// If not a package, try to resolve as a file
if (!collectionPath) {
try {
collectionPath = require.resolve(possibleCollectionPath, resolveOptions);
collectionPath = require.resolve(name, resolveOptions);
} catch (e) {
if ((e as NodeJS.ErrnoException).code !== 'MODULE_NOT_FOUND') {
throw e;
Expand Down

0 comments on commit 2a1bbc0

Please sign in to comment.