fix(pnp): always initialize the ESM loader when enabled by merceyz · Pull Request #3667 · yarnpkg/berry

merceyz · 2021-10-28T18:20:51Z

What's the problem this PR addresses?

Dynamic imports doesn't work in commonjs files when the entrypoint is also a commonjs module.

Fixes #3671
Fixes #3687
Fixes PnP support in the Babel repo (cc @nicolo-ribaudo)
Fixes (part of) the failing Angular e2e test

How did you fix it?

Update the ESM loader to handle extensionless files (workaround for node --loader treats entrypoint as ESM when should be loaded as CJS nodejs/node#33226), this allows removing the patch to require('module').runMain. Removing that patch lets Node initialize the loader regardless of the module type
Update process.mainModule when the commonjs module is already in the cache when loading the main module

Checklist

I have read the Contributing Guide.
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.

arcanis · 2021-11-03T10:37:15Z

-  );
-
-  test(
-    `it should always set default as module.exports when importing a commonjs file`,


Quick note: it's worth adding a (in-PR) comment when tests are removed, explaining why they aren't relevant anymore (were they bugged?) 😃

I documented it in the commit that removed them

These tests checked that the code generation was correct but the code generation was replaced with a patch to the `fs` bindings

timmywil · 2021-11-16T18:20:00Z

Could this be released to canary?

merceyz · 2021-11-16T18:37:58Z

Sure, running it now https://github.com/yarnpkg/berry/actions/runs/1468300994

Alternatively you can get it from sources

yarn set version from sources

teohhanhui · 2021-11-17T12:20:51Z

Looks like that has failed: https://github.com/yarnpkg/berry/runs/4229011500?check_suite_focus=true#step:7:1257

merceyz · 2021-11-17T13:05:46Z

The publish to the registry failed but set version... doesn't use the registry so it doesn't matter.

yarn set version canary

kherock · 2021-11-18T17:00:02Z

The extensionless file workaround for nodejs/node#33226 in this PR doesn't work when I'm attempting to load an entrypoint with an arbitrary file extension that's intended to be parsed by a require hook like babel-register. I have a local package that loads an entrypoint with a .ts extension that should normally load as CJS.

Are there any gotchas to renaming the ``: and `.js`: cases to the default: case? I expect .ts to load as CJS when the package doesn't declare "type": "module". This assumption might change once .mts exists, but for now defaulting to the package type feels like the most correct behavior.

merceyz · 2021-11-21T22:36:34Z

Could you open a seperate issue to track that?

Are there any gotchas to renaming the ``: and .js: cases to the default: case?

We don't want to deviate from how Node behaves more than we have to so we could only do that for the entrypoint

tavoyne · 2022-01-29T14:13:55Z

I'm experiencing an analogous issue that this PR doesn't seem to be fixing.

I'm trying to use the @mdx-js/loader@2.0.0-rc.2 webpack loader. In the package's entry point (it's a .cjs file), there's a dynamic import of an ES module that causes the following error:

node:internal/errors:464
    ErrorCaptureStackTrace(err);
    ^

Error [ERR_MODULE_NOT_FOUND]: Cannot find module '/Users/blackjelly/code/blog-3/.yarn/__virtual__/@mdx-js-loader-virtual-a537165eb1/0/cache/@mdx-js-loader-npm-2.0.0-rc.2-eb4487e27d-5637b8d92c.zip/node_modules/@mdx-js/loader/lib/index.js' imported from /Users/blackjelly/code/blog-3/.yarn/__virtual__/@mdx-js-loader-virtual-a537165eb1/0/cache/@mdx-js-loader-npm-2.0.0-rc.2-eb4487e27d-5637b8d92c.zip/node_modules/@mdx-js/loader/index.cjs

Which setting Yarn's version from sources or using canary doesn't fix.

merceyz · 2022-01-29T14:34:07Z

Please open a bug report with a reproduction

merceyz added the esm label Oct 28, 2021

merceyz mentioned this pull request Oct 29, 2021

[Bug?]: cannot import chai from a module using PNP #3671

Closed

1 task

merceyz force-pushed the merceyz/fix/pnp-esm branch 2 times, most recently from 170b32a to c920355 Compare October 29, 2021 20:33

merceyz mentioned this pull request Nov 2, 2021

[Bug?]: PnP fails to resolve dynamic require from CJS #3687

Closed

1 task

arcanis reviewed Nov 3, 2021

View reviewed changes

merceyz added 9 commits November 4, 2021 11:35

fix: handle extensionless commonjs entrypoint

2b351ac

fix: set the main module when the entrypoint is commonjs

c811644

test: use fixture

d6f38f6

test: fix check for unknown extensions

5ff0ad0

test: reuse fixture

9e77b18

test: remove old tests

acab291

These tests checked that the code generation was correct but the code generation was replaced with a patch to the `fs` bindings

chore: make the type stricter

4d269de

chore: versions

3ad4815

chore: changelog

d868c7f

merceyz force-pushed the merceyz/fix/pnp-esm branch from d314792 to d868c7f Compare November 4, 2021 10:36

arcanis merged commit 83311e1 into master Nov 10, 2021

arcanis deleted the merceyz/fix/pnp-esm branch November 10, 2021 09:54

timmywil mentioned this pull request Nov 10, 2021

12.0.2 regression: can no longer import uuid package (a package with exports) with yarn pnp vercel/next.js#31226

Closed

This was referenced Nov 21, 2021

Yarn berry PnP + Next.js 12 has import error. hoangvvo/next-connect#163

Closed

[Bug]: typescript patch fails to apply with typescript@4.5 #3722

Closed

daangnDylan mentioned this pull request Nov 22, 2021

yarn berry (3.1.0 cjs) cannot find '@stitches/react' stitchesjs/stitches#872

Closed

kherock mentioned this pull request Nov 22, 2021

[Bug?]: pnpEnableEsmLoader prevents most file extensions from loading as CJS #3782

Closed

1 task

enk0de mentioned this pull request Nov 28, 2021

Build is too slow(seems unable) when using yarn@3.2.0-rc.5 vercel/next.js#31880

Closed

kherock mentioned this pull request Dec 1, 2021

fix(pnp): load the entrypoint as CJS if it isn't ESM #3832

Merged

3 tasks

andreialecu mentioned this pull request Dec 22, 2021

TypeError [ERR_UNKNOWN_FILE_EXTENSION]: Unknown file extension "" for ... tsc #3900

Closed

merceyz mentioned this pull request Jan 6, 2022

Yarn PnP Support Status angular/angular-cli#16980

Open

yarnpkg locked as resolved and limited conversation to collaborators Jan 29, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix(pnp): always initialize the ESM loader when enabled#3667

fix(pnp): always initialize the ESM loader when enabled#3667
arcanis merged 9 commits into
masterfrom
merceyz/fix/pnp-esm

merceyz commented Oct 28, 2021 •

edited

Loading

Uh oh!

arcanis Nov 3, 2021 •

edited

Loading

Uh oh!

merceyz Nov 3, 2021 •

edited

Loading

Uh oh!

timmywil commented Nov 16, 2021

Uh oh!

merceyz commented Nov 16, 2021

Uh oh!

teohhanhui commented Nov 17, 2021

Uh oh!

merceyz commented Nov 17, 2021

Uh oh!

kherock commented Nov 18, 2021 •

edited

Loading

Uh oh!

merceyz commented Nov 21, 2021

Uh oh!

tavoyne commented Jan 29, 2022

Uh oh!

merceyz commented Jan 29, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

Uh oh!

Conversation

merceyz commented Oct 28, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

arcanis Nov 3, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

merceyz Nov 3, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

timmywil commented Nov 16, 2021

Uh oh!

merceyz commented Nov 16, 2021

Uh oh!

teohhanhui commented Nov 17, 2021

Uh oh!

merceyz commented Nov 17, 2021

Uh oh!

kherock commented Nov 18, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

merceyz commented Nov 21, 2021

Uh oh!

tavoyne commented Jan 29, 2022

Uh oh!

merceyz commented Jan 29, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

merceyz commented Oct 28, 2021 •

edited

Loading

arcanis Nov 3, 2021 •

edited

Loading

merceyz Nov 3, 2021 •

edited

Loading

kherock commented Nov 18, 2021 •

edited

Loading