fix(pnp): make sure that the package locator is fetched with a trailing slash (#6882)

zregvart · arcanis · commit fb0378827927 · 2019-01-14T15:09:57.000Z
* fix(pnp): make sure that the package locator is... ... fetched with a trailing slash I could not get P'n'P working on a project using `critical`[1] and traced it down to a lookup of a package locator without a trailing slash. A `require('resolve').sync('fg-loadcss')` from `inline-critical`[2] would fail to resolve as the `locatorsByLocations` lookup in `.pnp.js` would be using a key without a trailing slash, i.e.: ``` "../../.cache/yarn/v4/npm-inline-critical-4.0.7-3ffba6a869f39215c8bb00ed2dd6b872e7f98adc/node_modules/inline-critical" ``` `findPackageLocator` is invoked with `location` parameter (notice no trailing slash): ``` location = "/home/zregvart/.cache/yarn/v4/npm-inline-critical-4.0.7-3ffba6a869f39215c8bb00ed2dd6b872e7f98adc/node_modules/inline-critical" ``` from `resolveToUnqualified` with parameters: ``` request = "fg-loadcss/package.json" issuer = "/home/zregvart/.cache/yarn/v4/npm-inline-critical-4.0.7-3ffba6a869f39215c8bb00ed2dd6b872e7f98adc/node_modules/inline-critical" ``` All starting from `loadNodeModulesSync` in `resolve`'s `sync.js`[3] that ends up having `absoluteStart` without the trailing slash. Not sure if this is the most correct way of fixing this issue, by just ensuring that the trailing slash needed for the lookup is added if missing. All tests from `yarn test` and from `packages/pkg-tests` `yarn jest yarn.test.js` pass, so it seems like a start. This is how to reproduce: package.json: ```json { "name": "require-failure", "version": "1.0.0", "main": "index.js", "license": "MIT", "scripts": { "install": "node --require ./.pnp.js index.js" }, "devDependencies": { "critical": "^1.3.4" }, "installConfig": { "pnp": true } } ``` index.js: ```javascript require('critical').generate({ inline: true, html: '<!DOCTYPE html><html><head><link rel="stylesheet" href="site.css"></head><body><h1>Hello</h1></body></html>' }); ``` site.css: ```css h1 { font-weight: bold; } ``` Not sure how to create a package test from that example (sorry). [1] https://github.com/addyosmani/critical [2] https://github.com/bezoerb/inline-critical/blob/340db21f6a2454ed74ede7a0b317d4c4e177770d/index.js#L32 [3] https://github.com/browserify/resolve/blob/254bb4029df2f8d20a33043dfabd8e5cabfa37df/lib/sync.js#L52 * Update CHANGELOG.md
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -20,6 +20,10 @@ Please add one entry in this file for each change in Yarn's behavior. Use the sa
 
   [#6912](https://github.com/yarnpkg/yarn/pull/6912) - [**Billy Vong**](https://github.com/billyvg)
 
+- Fixes an issue where `resolve` would forward an incomplete basedir to the PnP hook
+
+  [#6882](https://github.com/yarnpkg/yarn/pull/6882) - [**Zoran Regvart**](https://github.com/zregvart)
+
 ## 1.13.0
 
 - Implements a new `package.json` field: `peerDependenciesMeta`
diff --git a/src/util/generate-pnp-map-api.tpl.js b/src/util/generate-pnp-map-api.tpl.js
@@ -773,6 +773,10 @@ exports.setupCompatibilityLayer = () => {
           // Extract the name of the package being requested (1=full name, 2=scope name, 3=local name)
           const parts = request.match(/^((?:(@[^\/]+)\/)?([^\/]+))/);
 
+          // make sure that basedir ends with a slash
+          if (basedir.charAt(basedir.length - 1) !== '/') {
+            basedir = path.join(basedir, '/');
+          }
           // This is guaranteed to return the path to the "package.json" file from the given package
           const manifestPath = exports.resolveToUnqualified(`${parts[1]}/package.json`, basedir);
 
