From ee7bf5d0b43305edb78428d4d68b94be0ef74798 Mon Sep 17 00:00:00 2001 From: Zoran Regvart Date: Mon, 7 Jan 2019 17:17:12 +0100 Subject: [PATCH 1/2] 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: '

Hello

' }); ``` 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 --- src/util/generate-pnp-map-api.tpl.js | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/util/generate-pnp-map-api.tpl.js b/src/util/generate-pnp-map-api.tpl.js index a703bcb9b7..835b6f85f1 100644 --- 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); From 1aeae1eef0df6983ad1857dcd0c8abb9feb955f4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ma=C3=ABl=20Nison?= Date: Mon, 14 Jan 2019 15:09:03 +0000 Subject: [PATCH 2/2] Update CHANGELOG.md --- CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 14ec287b01..ff4ff51992 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,10 @@ Please add one entry in this file for each change in Yarn's behavior. Use the sa [#6878](https://github.com/yarnpkg/yarn/pull/6878) - [**Maël Nison**](https://twitter.com/arcanis) +- 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`