Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Builtin pnp resolver #162

Open
arcanis opened this issue Dec 2, 2018 · 11 comments

Comments

Projects
None yet
4 participants
@arcanis
Copy link

commented Dec 2, 2018

I've wrote the following plugin to make it easy to use PnP with Webpack. Would you be interested to move it in enhanced-resolve in a later version? I think this would prove useful from a DevX perspective, one less configuration required, which would likely help adoption.

The only one issue I'm aware of is arcanis/pnp-webpack-plugin#3, but I'm confident a solution can be easily found (we actually already have one, the main issue is that it suppresses the semantic errors that PnP can throw).

@arcanis

This comment has been minimized.

Copy link
Author

commented Jan 11, 2019

Ping @sokra @TheLarkInn, at least to start a discussion? 😃

@TheLarkInn

This comment has been minimized.

Copy link
Member

commented Jan 21, 2019

Wow sorry this repo is watched a lot less. I can take a look at this tomorrow!

@arcanis

This comment has been minimized.

Copy link
Author

commented Jan 21, 2019

No worry, thanks! 😃👍

@arcanis arcanis referenced a pull request that will close this issue Jan 28, 2019

Open

Adds support for PnP #168

@arcanis

This comment has been minimized.

Copy link
Author

commented Jan 28, 2019

@TheLarkInn I went ahead and put up a PR at #168 😃

@arcanis

This comment has been minimized.

Copy link
Author

commented Feb 13, 2019

I appreciate that we're all busy, but together with #168 it's been two months this issue has been opened with little to no feedback - I can only guess that you're unsatisfied with something, but unless you express it clearly it will be hard for me to adress your concerns in a productive way.

@sokra

This comment has been minimized.

Copy link
Member

commented Feb 16, 2019

@arcanis sorry I'm in parental leave currently. This would be a breaking change anyway, so no need to hurry, since it would be webpack 5 anyway.

@arcanis

This comment has been minimized.

Copy link
Author

commented Feb 16, 2019

I see, no worries then, it certainly can wait! Thanks for the update 😊

@arcanis

This comment has been minimized.

Copy link
Author

commented Mar 23, 2019

Coming to the news! Since the last time Gatsby and Jest (and some less-known tools) now have builtin support, and Parcel signaled interest. Additionally we've announced Yarn 2, which will use PnP by default. We would appreciate a ton any help ensuring a smooth migration path to our user 🙂

@evilebottnawi

This comment has been minimized.

Copy link
Member

commented Mar 23, 2019

Thanks for helping us 👍

@sokra

This comment has been minimized.

Copy link
Member

commented Mar 23, 2019

I have a few questions:

  • How to make pnp resolve to the module package.json field instead of the main field?
  • How to make pnp handle aliasings in the package.json like with the browser field?
  • How to get the nearest package.json for meta infomation like sideEffects
@arcanis

This comment has been minimized.

Copy link
Author

commented Mar 24, 2019

How to make pnp resolve to the module package.json field instead of the main field?
How to make pnp handle aliasing in the package.json like with the browser field?

Short answer: you don't 😛

The PnP API exposes three resolution functions, but only one is required. The other two are mostly here for convenience for when the resolution pipeline only allow to replace the resolution on a "all or nothing" basis. This isn't the case of enhanced-resolve, which allow to inject plugins at various parts of the resolution.

So in your case, you'd only need the resolveToUnqualified function, which will resolve bare identifiers to what we call unqualified paths (/cache/lodash-1.0.0). Because it doesn't resolve folders, extensions, main fields, etc, this function never has to query the filesystem; all the data it needs have already been generated at install time.

In fact, if you look at my PR, you'll see that it's exactly what I do: the plugin is injected where enhanced-resolve would typically resolve lodash to /app/node_modules/lodash, and everything else (extensions, main field, index.js, ...) is left to the individual followup plugins like MainFieldPlugin.

How to get the nearest package.json for meta information like sideEffect

If you point me to the location where you have this logic implemented I probably can be more precise, but there are two ways.

The hacky way is that you don't change anything. Assuming that you find the nearest node_modules part in the path and obtain the package.json from there, it will continue to work. Yarn installs its cache in such a way that the node_modules naming convention is preserved (so the cache paths are actually a bit like /cache/lodash-1.0.0/node_modules/lodash instead of the cleaner /cache/lodash-1.0.0). It's kind of an implementation detail and it would be nice not to rely on it, but I don't plan to change it anytime soon.

The correct way would be to simply ask the PnP API to tell you what package owns a file, and where is located its root:

let nearestPackageJson;

if (process.versions.pnp) {
  const {findPackageLocator, getPackageInformation} = require(`pnpapi`);

  const filePath = `/cache/lodash-1.0.0/foo/bar/hello/world.js`;

  const ownerPackage = findPackageLocator(filePath); 
  const {packageLocation} = getPackageInformation(ownerPackage);

  nearestPackageJson = `${packageLocation}/package.json`;
}

if (!nearestPackageJson) {
  // ... regular logic you apply
}
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.