Skip to content

Conversation

arcanis
Copy link
Member

@arcanis arcanis commented Jan 19, 2023

What's the problem this PR addresses?

It's not currently possible to obtain the PnP API from import 'pnpapi'.

How did you fix it?

Avoid checking if legacy resolve is allowed for the pnpapi.

Checklist

  • I have set the packages that need to be released for my changes to be effective.
  • I will check that all automated PR checks pass before the PR gets reviewed.

@merceyz merceyz self-requested a review January 19, 2023 14:35
Copy link
Member

@merceyz merceyz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This solution doesn't seem correct to me, you can solve the issue with the following diff:

diff --git a/packages/yarnpkg-pnp/sources/esm-loader/hooks/resolve.ts b/packages/yarnpkg-pnp/sources/esm-loader/hooks/resolve.ts
index 8a9646f9e..a49e409f5 100644
--- a/packages/yarnpkg-pnp/sources/esm-loader/hooks/resolve.ts
+++ b/packages/yarnpkg-pnp/sources/esm-loader/hooks/resolve.ts
@@ -88,7 +88,7 @@ export async function resolve(
 
     // If the package.json doesn't list an `exports` field, Node will tolerate omitting the extension
     // https://github.com/nodejs/node/blob/0996eb71edbd47d9f9ec6153331255993fd6f0d1/lib/internal/modules/esm/resolve.js#L686-L691
-    if (subPath === ``) {
+    if (dependencyName !== `pnpapi` && subPath === ``) {
       const resolved = pnpapi.resolveToUnqualified(`${dependencyName}/package.json`, issuer);
       if (resolved) {
         const content = await loaderUtils.tryReadFile(resolved);

@arcanis arcanis requested a review from merceyz January 19, 2023 15:05
@arcanis
Copy link
Member Author

arcanis commented Jan 19, 2023

Yes indeed, somehow forgot that we could just resolve to the PnP file and let Node expose it 😄

@merceyz merceyz changed the title Fixes importing the PnP API from ESM modules Fixes importing the PnP API from ES modules Jan 19, 2023
@merceyz merceyz merged commit 94e4069 into master Jan 19, 2023
@merceyz merceyz deleted the mael/esm-pnp-api branch January 19, 2023 17:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants