Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
fix(core): hangs when used with yarn PnP
The `findNpmPackage` function in `runtime-info.ts` has a subtle and mostly innocuous bug. Its parent, `collectRuntimeInformation`, iterates over the modules in `require.cache` and resolves the library version for each. It uses the `paths` property of each entry to locate the package description for that module. An example: For `/foo/bar/aws-cdk/packages/aws-cdk/lib/index.js`: ```js [ '/foo/aws-cdk/packages/aws-cdk/lib/node_modules', '/foo/aws-cdk/packages/aws-cdk/node_modules', '/foo/aws-cdk/packages/node_modules', '/foo/aws-cdk/node_modules', '/foo/node_modules', '/node_modules' ] ``` `findNpmPackage` maps over these, and if the string ends with `/node_modules` or `\\node_modules` it strips that off, ending with: ```js [ '/foo/aws-cdk/packages/aws-cdk/lib', '/foo/aws-cdk/packages/aws-cdk', '/foo/aws-cdk/packages', '/foo/aws-cdk', '/foo', '' ] ``` ...and passes this array as the `paths` option to `require.resolve`. The problem is that `''` is interpreted as the current working directory, and instead of walking the file tree to the root (as was probably intended) it appends whatever node's current working directory is to the search. It's hard to imagine this ever really having a negative impact on package resolution, but unfortunately this triggered a likewise obscure bug in yarn 2 when configured with PnP. (See yarnpkg/berry#1298). This bug is fixed in yarn 2 master but hasn't been released yet. The effect of these two bugs when triggered is for the CDK CLI to hang. This change seems to meet the intent of the code without having the above problem. It seems canonically correct and uses the `fs` lib more idiomatically. It's also less code :) This passes all existing tests. It's not clear how this would be tested without exporting `findNpmPackage` and adding tests for it. **CAVEAT:** I can't test this on windows, but it's worth noting the following: ```js // Node 12.16.3 path.win32.resolve('C:') // => 'C:\\foo\\aws-cdk' (current working directory) ``` ```js // Node 10.13.0 path.win32.resolve('C:') // => 'C:\\' ``` Both of these results come on OSX. But it appears that the yarn bug would not trigger on either version, while node may or may not have the same wrong behavior for `findNpmVersion` on windows.
- Loading branch information