From b24b62ea8a24247683635070e9f1e86e22f7d7fd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ma=C3=ABl=20Nison?= Date: Mon, 28 Jan 2019 15:10:33 +0000 Subject: [PATCH 1/6] Adds support for PnP --- lib/PnpPlugin.js | 49 +++++++++++++++++ lib/ResolverFactory.js | 13 ++++- test/fixtures/pnp/pkg/dir/index.js | 0 test/fixtures/pnp/pkg/index.js | 0 test/fixtures/pnp/pkg/symlink | 1 + test/fixtures/pnp/pkg/typescript/index.ts | 0 test/pnp.js | 67 +++++++++++++++++++++++ 7 files changed, 129 insertions(+), 1 deletion(-) create mode 100644 lib/PnpPlugin.js create mode 100644 test/fixtures/pnp/pkg/dir/index.js create mode 100644 test/fixtures/pnp/pkg/index.js create mode 120000 test/fixtures/pnp/pkg/symlink create mode 100644 test/fixtures/pnp/pkg/typescript/index.ts create mode 100644 test/pnp.js diff --git a/lib/PnpPlugin.js b/lib/PnpPlugin.js new file mode 100644 index 00000000..0a6c9d91 --- /dev/null +++ b/lib/PnpPlugin.js @@ -0,0 +1,49 @@ +"use strict"; + +const path = require("path"); + +module.exports = class PnpPlugin { + constructor(source, pnpApi, target) { + this.source = source; + this.pnpApi = pnpApi; + this.target = target; + } + + apply(resolver) { + const target = resolver.ensureHook(this.target); + resolver + .getHook(this.source) + .tapAsync("PnpPlugin", (requestContext, resolveContext, callback) => { + let request = requestContext.request; + let issuer = requestContext.context.issuer; + + // When using require.context, issuer seems to be false (cf https://github.com/webpack/webpack-dev-server/blob/d0725c98fb752d8c0b1e8c9067e526e22b5f5134/client-src/default/index.js#L94) + if (!issuer) issuer = `${requestContext.path}/`; + + // We only support issuer when they're absolute paths. I'm not sure the opposite can ever happen, but better check here. + if (!path.isAbsolute(issuer)) + throw new Error( + `Cannot successfully resolve this dependency - issuer not supported (${issuer})` + ); + + let resolution; + try { + resolution = this.pnpApi.resolveToUnqualified(request, issuer, { + considerBuiltins: false + }); + } catch (error) { + return callback(error); + } + + resolver.doResolve( + target, + Object.assign({}, requestContext, { + request: resolution + }), + null, + resolveContext, + callback + ); + }); + } +}; diff --git a/lib/ResolverFactory.js b/lib/ResolverFactory.js index d148413c..8388f69c 100644 --- a/lib/ResolverFactory.js +++ b/lib/ResolverFactory.js @@ -28,6 +28,7 @@ const AppendPlugin = require("./AppendPlugin"); const ResultPlugin = require("./ResultPlugin"); const ModuleAppendPlugin = require("./ModuleAppendPlugin"); const UnsafeCachePlugin = require("./UnsafeCachePlugin"); +const PnpPlugin = require("./PnpPlugin"); exports.createResolver = function(options) { //// OPTIONS //// @@ -97,6 +98,14 @@ exports.createResolver = function(options) { // Use only the sync constiants of the file system calls const useSyncFileSystemCalls = options.useSyncFileSystemCalls; + // A PnP API that should be used - null is "never", undefined is "auto" + const pnpApi = + typeof options.pnpApi === "undefined" + ? process.versions.pnp + ? require("pnpapi") // eslint-disable-line node/no-missing-require + : null + : options.pnpApi; + // A prepared Resolver to which the plugins are attached let resolver = options.resolver; @@ -215,6 +224,7 @@ exports.createResolver = function(options) { plugins.push(new TryNextPlugin("raw-module", null, "module")); // module + if (pnpApi) plugins.push(new PnpPlugin("before-module", pnpApi, "resolve")); modules.forEach(item => { if (Array.isArray(item)) plugins.push( @@ -280,7 +290,8 @@ exports.createResolver = function(options) { aliasFields.forEach(item => { plugins.push(new AliasFieldPlugin("file", item, "resolve")); }); - if (symlinks) plugins.push(new SymlinkPlugin("file", "relative")); + if (symlinks && !pnpApi) + plugins.push(new SymlinkPlugin("file", "relative")); plugins.push(new FileExistsPlugin("file", "existing-file")); // existing-file diff --git a/test/fixtures/pnp/pkg/dir/index.js b/test/fixtures/pnp/pkg/dir/index.js new file mode 100644 index 00000000..e69de29b diff --git a/test/fixtures/pnp/pkg/index.js b/test/fixtures/pnp/pkg/index.js new file mode 100644 index 00000000..e69de29b diff --git a/test/fixtures/pnp/pkg/symlink b/test/fixtures/pnp/pkg/symlink new file mode 120000 index 00000000..87245193 --- /dev/null +++ b/test/fixtures/pnp/pkg/symlink @@ -0,0 +1 @@ +dir \ No newline at end of file diff --git a/test/fixtures/pnp/pkg/typescript/index.ts b/test/fixtures/pnp/pkg/typescript/index.ts new file mode 100644 index 00000000..e69de29b diff --git a/test/pnp.js b/test/pnp.js new file mode 100644 index 00000000..cdc08305 --- /dev/null +++ b/test/pnp.js @@ -0,0 +1,67 @@ +var path = require("path"); +require("should"); +var ResolverFactory = require("../lib/ResolverFactory"); +var NodeJsInputFileSystem = require("../lib/NodeJsInputFileSystem"); +var CachedInputFileSystem = require("../lib/CachedInputFileSystem"); + +var pnpApi = { + mocks: new Map(), + resolveToUnqualified(request, issuer) { + if (pnpApi.mocks.has(request)) { + return pnpApi.mocks.get(request); + } else { + throw new Error(`No way`); + } + } +}; + +var nodeFileSystem = new CachedInputFileSystem( + new NodeJsInputFileSystem(), + 4000 +); + +var resolver = ResolverFactory.createResolver({ + extensions: [".ts", ".js"], + fileSystem: nodeFileSystem, + pnpApi +}); + +var fixture = path.resolve(__dirname, "fixtures", "pnp"); + +describe("extensions", function() { + it("should resolve by going through the pnp api", function(done) { + pnpApi.mocks.set( + "pkg/dir/index.js", + path.resolve(fixture, "pkg/dir/index.js") + ); + resolver.resolve({}, fixture, "pkg/dir/index.js", {}, function( + err, + result + ) { + console.log(err); + result.should.equal(path.resolve(fixture, "pkg/dir/index.js")); + done(); + }); + }); + it("should resolve module names", function(done) { + pnpApi.mocks.set("pkg", path.resolve(fixture, "pkg")); + resolver.resolve({}, fixture, "pkg", {}, function(err, result) { + result.should.equal(path.resolve(fixture, "pkg/index.js")); + done(); + }); + }); + it("should not resolve symlinks", function(done) { + pnpApi.mocks.set("pkg/symlink", path.resolve(fixture, "pkg/symlink")); + resolver.resolve({}, fixture, "pkg/symlink", {}, function(err, result) { + result.should.equal(path.resolve(fixture, "pkg/symlink/index.js")); + done(); + }); + }); + it("should properly deal with other extensions", function(done) { + pnpApi.mocks.set("pkg/typescript", path.resolve(fixture, "pkg/typescript")); + resolver.resolve({}, fixture, "pkg/typescript", {}, function(err, result) { + result.should.equal(path.resolve(fixture, "pkg/typescript/index.ts")); + done(); + }); + }); +}); From 1af390658def8e48b047746a8b88ce77df9f9300 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ma=C3=ABl=20Nison?= Date: Tue, 26 Mar 2019 14:24:20 +0000 Subject: [PATCH 2/6] Uses requestContext.path over requestContext.context --- lib/PnpPlugin.js | 13 +++---------- 1 file changed, 3 insertions(+), 10 deletions(-) diff --git a/lib/PnpPlugin.js b/lib/PnpPlugin.js index 0a6c9d91..6de8d154 100644 --- a/lib/PnpPlugin.js +++ b/lib/PnpPlugin.js @@ -15,16 +15,9 @@ module.exports = class PnpPlugin { .getHook(this.source) .tapAsync("PnpPlugin", (requestContext, resolveContext, callback) => { let request = requestContext.request; - let issuer = requestContext.context.issuer; - - // When using require.context, issuer seems to be false (cf https://github.com/webpack/webpack-dev-server/blob/d0725c98fb752d8c0b1e8c9067e526e22b5f5134/client-src/default/index.js#L94) - if (!issuer) issuer = `${requestContext.path}/`; - - // We only support issuer when they're absolute paths. I'm not sure the opposite can ever happen, but better check here. - if (!path.isAbsolute(issuer)) - throw new Error( - `Cannot successfully resolve this dependency - issuer not supported (${issuer})` - ); + + // The trailing slash indicates to PnP that this value is a folder rather than a file + let issuer = `${requestContext.path}/`; let resolution; try { From f5ce5a690bb63591361aee708d93ba6dc6dc848b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ma=C3=ABl=20Nison?= Date: Tue, 26 Mar 2019 14:27:28 +0000 Subject: [PATCH 3/6] Fixes linting --- lib/PnpPlugin.js | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/lib/PnpPlugin.js b/lib/PnpPlugin.js index 6de8d154..1b1f84f4 100644 --- a/lib/PnpPlugin.js +++ b/lib/PnpPlugin.js @@ -1,7 +1,5 @@ "use strict"; -const path = require("path"); - module.exports = class PnpPlugin { constructor(source, pnpApi, target) { this.source = source; @@ -15,8 +13,8 @@ module.exports = class PnpPlugin { .getHook(this.source) .tapAsync("PnpPlugin", (requestContext, resolveContext, callback) => { let request = requestContext.request; - - // The trailing slash indicates to PnP that this value is a folder rather than a file + + // The trailing slash indicates to PnP that this value is a folder rather than a file let issuer = `${requestContext.path}/`; let resolution; From 59a1c45f9bf2235952c22b77cb7f3ca5ee2cf3a4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ma=C3=ABl=20Nison?= Date: Tue, 26 Mar 2019 14:29:10 +0000 Subject: [PATCH 4/6] Adds a comment about symlinks --- lib/ResolverFactory.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/lib/ResolverFactory.js b/lib/ResolverFactory.js index 8388f69c..3b4117b4 100644 --- a/lib/ResolverFactory.js +++ b/lib/ResolverFactory.js @@ -290,6 +290,8 @@ exports.createResolver = function(options) { aliasFields.forEach(item => { plugins.push(new AliasFieldPlugin("file", item, "resolve")); }); + // PnP currently needs symlinks to be preserved to resolve peer dependencies + // Ref: https://github.com/webpack/enhanced-resolve/pull/168/files#r268819172 if (symlinks && !pnpApi) plugins.push(new SymlinkPlugin("file", "relative")); plugins.push(new FileExistsPlugin("file", "existing-file")); From 816e48a6c3779b2f2a82bc28252ff4e5f32156f9 Mon Sep 17 00:00:00 2001 From: Tobias Koppers Date: Mon, 8 Apr 2019 20:22:47 +0200 Subject: [PATCH 5/6] convert let to const --- lib/PnpPlugin.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/PnpPlugin.js b/lib/PnpPlugin.js index 1b1f84f4..f4eb8bf8 100644 --- a/lib/PnpPlugin.js +++ b/lib/PnpPlugin.js @@ -12,10 +12,10 @@ module.exports = class PnpPlugin { resolver .getHook(this.source) .tapAsync("PnpPlugin", (requestContext, resolveContext, callback) => { - let request = requestContext.request; + const request = requestContext.request; // The trailing slash indicates to PnP that this value is a folder rather than a file - let issuer = `${requestContext.path}/`; + const issuer = `${requestContext.path}/`; let resolution; try { From 912a6ef8757fef7d5dc2c22ed6e92bf6858e7e5e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ma=C3=ABl=20Nison?= Date: Thu, 18 Apr 2019 15:22:14 +0100 Subject: [PATCH 6/6] Allows to enforce symlink resolution even w/ pnp --- lib/ResolverFactory.js | 29 ++++++++++++++++------------- 1 file changed, 16 insertions(+), 13 deletions(-) diff --git a/lib/ResolverFactory.js b/lib/ResolverFactory.js index 3b4117b4..6ca8cd34 100644 --- a/lib/ResolverFactory.js +++ b/lib/ResolverFactory.js @@ -68,9 +68,23 @@ exports.createResolver = function(options) { // A list of module alias configurations or an object which maps key to value let alias = options.alias || []; + // A PnP API that should be used - null is "never", undefined is "auto" + const pnpApi = + typeof options.pnpApi === "undefined" + ? process.versions.pnp + ? require("pnpapi") // eslint-disable-line node/no-missing-require + : null + : options.pnpApi; + // Resolve symlinks to their symlinked location const symlinks = - typeof options.symlinks !== "undefined" ? options.symlinks : true; + typeof options.symlinks !== "undefined" + ? options.symlinks + : // PnP currently needs symlinks to be preserved to resolve peer dependencies + // Ref: https://github.com/webpack/enhanced-resolve/pull/168/files#r268819172 + pnpApi + ? false + : true; // Resolve to a context instead of a file const resolveToContext = options.resolveToContext || false; @@ -98,14 +112,6 @@ exports.createResolver = function(options) { // Use only the sync constiants of the file system calls const useSyncFileSystemCalls = options.useSyncFileSystemCalls; - // A PnP API that should be used - null is "never", undefined is "auto" - const pnpApi = - typeof options.pnpApi === "undefined" - ? process.versions.pnp - ? require("pnpapi") // eslint-disable-line node/no-missing-require - : null - : options.pnpApi; - // A prepared Resolver to which the plugins are attached let resolver = options.resolver; @@ -290,10 +296,7 @@ exports.createResolver = function(options) { aliasFields.forEach(item => { plugins.push(new AliasFieldPlugin("file", item, "resolve")); }); - // PnP currently needs symlinks to be preserved to resolve peer dependencies - // Ref: https://github.com/webpack/enhanced-resolve/pull/168/files#r268819172 - if (symlinks && !pnpApi) - plugins.push(new SymlinkPlugin("file", "relative")); + if (symlinks) plugins.push(new SymlinkPlugin("file", "relative")); plugins.push(new FileExistsPlugin("file", "existing-file")); // existing-file