diff --git a/CHANGELOG.md b/CHANGELOG.md index f9d4f6a6b2..4383f84586 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -28,6 +28,10 @@ Please add one entry in this file for each change in Yarn's behavior. Use the sa [#6712](https://github.com/yarnpkg/yarn/pull/6712) - [**Maël Nison**](https://twitter.com/arcanis) +- Adds transparent support for the [`resolve`](https://github.com/browserify/resolve) package when using Plug'n'Play + + [#6816](https://github.com/yarnpkg/yarn/pull/6816) - [**Maël Nison**](https://twitter.com/arcanis) + - Properly reports the error codes when the npm registry throws 500's [#6817](https://github.com/yarnpkg/yarn/pull/6817) - [**Maël Nison**](https://twitter.com/arcanis) diff --git a/packages/pkg-tests/pkg-tests-specs/sources/pnp.js b/packages/pkg-tests/pkg-tests-specs/sources/pnp.js index 8efd0028c1..a4ed85795d 100644 --- a/packages/pkg-tests/pkg-tests-specs/sources/pnp.js +++ b/packages/pkg-tests/pkg-tests-specs/sources/pnp.js @@ -1432,5 +1432,28 @@ module.exports = makeTemporaryEnv => { }); }), ); + + test( + `it should transparently support the "resolve" package`, + makeTemporaryEnv( + { + dependencies: { + [`resolve`]: `https://github.com/browserify/resolve.git`, + }, + resolutions: { + [`path-parse`]: `https://registry.yarnpkg.com/path-parse/-/path-parse-1.0.6.tgz`, + }, + }, + {plugNPlay: true}, + async ({path, run, source}) => { + await run(`install`); + + await expect(source(`require('resolve').sync('resolve')`)).resolves.toEqual( + await source(`require.resolve('resolve')`), + ); + }, + ), + 15000, + ); }); }; diff --git a/src/util/generate-pnp-map-api.tpl.js b/src/util/generate-pnp-map-api.tpl.js index b3280459c6..8236b4332d 100644 --- a/src/util/generate-pnp-map-api.tpl.js +++ b/src/util/generate-pnp-map-api.tpl.js @@ -20,7 +20,7 @@ const topLevelLocator = {name: null, reference: null}; const blacklistedLocator = {name: NaN, reference: NaN}; // Used for compatibility purposes - cf setupCompatibilityLayer -const patchedModules = new Map(); +const patchedModules = []; const fallbackLocators = [topLevelLocator]; // Matches backslashes of Windows paths @@ -648,8 +648,10 @@ exports.setup = function setup() { // Some modules might have to be patched for compatibility purposes - if (patchedModules.has(request)) { - module.exports = patchedModules.get(request)(module.exports); + for (const [filter, patchFn] of patchedModules) { + if (filter.test(request)) { + module.exports = patchFn(exports.findPackageLocator(parent.filename), module.exports); + } } return module.exports; @@ -733,17 +735,6 @@ exports.setup = function setup() { }; exports.setupCompatibilityLayer = () => { - // see https://github.com/browserify/resolve/blob/master/lib/caller.js - const getCaller = () => { - const origPrepareStackTrace = Error.prepareStackTrace; - - Error.prepareStackTrace = (_, stack) => stack; - const stack = new Error().stack; - Error.prepareStackTrace = origPrepareStackTrace; - - return stack[2].getFileName(); - }; - // ESLint currently doesn't have any portable way for shared configs to specify their own // plugins that should be used (https://github.com/eslint/eslint/issues/10125). This will // likely get fixed at some point, but it'll take time and in the meantime we'll just add @@ -758,84 +749,48 @@ exports.setupCompatibilityLayer = () => { } } - // We need to shim the "resolve" module, because Liftoff uses it in order to find the location - // of the module in the dependency tree. And Liftoff is used to power Gulp, which doesn't work - // at all unless modulePath is set, which we cannot configure from any other way than through - // the Liftoff pipeline (the key isn't whitelisted for env or cli options). + // Modern versions of `resolve` support a specific entry point that custom resolvers can use + // to inject a specific resolution logic without having to patch the whole package. + // + // Cf: https://github.com/browserify/resolve/pull/174 - patchedModules.set('resolve', realResolve => { - const mustBeShimmed = caller => { - const callerLocator = exports.findPackageLocator(caller); - - return callerLocator && callerLocator.name === 'liftoff'; - }; - - const attachCallerToOptions = (caller, options) => { - if (!options.basedir) { - options.basedir = path.dirname(caller); + patchedModules.push([ + /^\.\/normalize-options\.js$/, + (issuer, normalizeOptions) => { + if (!issuer || issuer.name !== 'resolve') { + return normalizeOptions; } - }; - const resolveSyncShim = (request, {basedir}) => { - return exports.resolveRequest(request, basedir, { - considerBuiltins: false, - }); - }; + return (request, opts) => { + opts = opts || {}; - const resolveShim = (request, options, callback) => { - setImmediate(() => { - let error; - let result; - - try { - result = resolveSyncShim(request, options); - } catch (thrown) { - error = thrown; + if (opts.forceNodeResolution) { + return opts; } - callback(error, result); - }); - }; - - return Object.assign( - (request, options, callback) => { - if (typeof options === 'function') { - callback = options; - options = {}; - } else if (!options) { - options = {}; - } + opts.preserveSymlinks = true; + opts.paths = function(request, basedir, getNodeModulesDir, opts) { + // Extract the name of the package being requested (1=full name, 2=scope name, 3=local name) + const parts = request.match(/^((?:(@[^\/]+)\/)?([^\/]+))/); - const caller = getCaller(); - attachCallerToOptions(caller, options); + // 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); - if (mustBeShimmed(caller)) { - return resolveShim(request, options, callback); - } else { - return realResolve.sync(request, options, callback); - } - }, - { - sync: (request, options) => { - if (!options) { - options = {}; + // The first dirname strips the package.json, the second strips the local named folder + let nodeModules = path.dirname(path.dirname(manifestPath)); + + // Strips the scope named folder if needed + if (parts[2]) { + nodeModules = path.dirname(nodeModules); } - const caller = getCaller(); - attachCallerToOptions(caller, options); + return [nodeModules]; + }; - if (mustBeShimmed(caller)) { - return resolveSyncShim(request, options); - } else { - return realResolve.sync(request, options); - } - }, - isCore: request => { - return realResolve.isCore(request); - }, - }, - ); - }); + return opts; + }; + }, + ]); }; if (module.parent && module.parent.id === 'internal/preload') {