Skip to content

Commit

Permalink
fix(pnp): support require(esm) (#6639)
Browse files Browse the repository at this point in the history
**What's the problem this PR addresses?**

Node.js supports `require(esm)` now so we can avoid throwing an error if
support is enabled.

Fixes #6336
Ref nodejs/citgm#1086

**How did you fix it?**

Avoid throwing `ERR_REQUIRE_ESM` if `process.features.require_module` is
truthy.

**Checklist**
- [x] I have read the [Contributing
Guide](https://yarnpkg.com/advanced/contributing).
- [x] I have set the packages that need to be released for my changes to
be effective.
- [x] I will check that all automated PR checks pass before the PR gets
reviewed.
  • Loading branch information
merceyz authored Dec 28, 2024
1 parent 8bfe2d5 commit 3978649
Show file tree
Hide file tree
Showing 5 changed files with 116 additions and 26 deletions.
24 changes: 13 additions & 11 deletions .pnp.cjs

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

27 changes: 27 additions & 0 deletions .yarn/versions/9630a917.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
releases:
"@yarnpkg/cli": patch
"@yarnpkg/plugin-pnp": patch
"@yarnpkg/pnp": patch

declined:
- "@yarnpkg/plugin-compat"
- "@yarnpkg/plugin-constraints"
- "@yarnpkg/plugin-dlx"
- "@yarnpkg/plugin-essentials"
- "@yarnpkg/plugin-init"
- "@yarnpkg/plugin-interactive-tools"
- "@yarnpkg/plugin-nm"
- "@yarnpkg/plugin-npm-cli"
- "@yarnpkg/plugin-pack"
- "@yarnpkg/plugin-patch"
- "@yarnpkg/plugin-pnpm"
- "@yarnpkg/plugin-stage"
- "@yarnpkg/plugin-typescript"
- "@yarnpkg/plugin-version"
- "@yarnpkg/plugin-workspace-tools"
- "@yarnpkg/builder"
- "@yarnpkg/core"
- "@yarnpkg/doctor"
- "@yarnpkg/nm"
- "@yarnpkg/pnpify"
- "@yarnpkg/sdks"
62 changes: 60 additions & 2 deletions packages/acceptance-tests/pkg-tests-specs/sources/pnp-esm.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -613,7 +613,8 @@ describe(`Plug'n'Play - ESM`, () => {
),
);

test(
// @ts-expect-error - Missing types
(process.features.require_module ? it.skip : it)(
`it should throw ERR_REQUIRE_ESM when requiring a file with type=module`,
makeTemporaryEnv(
{
Expand Down Expand Up @@ -643,7 +644,36 @@ describe(`Plug'n'Play - ESM`, () => {
),
);

test(
// @ts-expect-error - Missing types
(process.features.require_module ? it : it.skip)(
`it should not throw ERR_REQUIRE_ESM when requiring a file with type=module`,
makeTemporaryEnv(
{
dependencies: {
'no-deps-esm': `1.0.0`,
},
},
{
pnpEnableEsmLoader: true,
},
async ({path, run, source}) => {
await run(`install`);

await xfs.writeFilePromise(ppath.join(path, `index.js`), `
require('no-deps-esm')
console.log('ESM required')
`);

await expect(run(`node`, `index.js`)).resolves.toMatchObject({
code: 0,
stdout: `ESM required\n`,
});
},
),
);

// @ts-expect-error - Missing types
(process.features.require_module ? it.skip : it)(
`it should throw ERR_REQUIRE_ESM when requiring a .mjs file`,
makeTemporaryEnv(
{
Expand Down Expand Up @@ -673,6 +703,34 @@ describe(`Plug'n'Play - ESM`, () => {
),
);

// @ts-expect-error - Missing types
(process.features.require_module ? it : it.skip)(
`it should not throw ERR_REQUIRE_ESM when requiring a .mjs file`,
makeTemporaryEnv(
{
dependencies: {
'no-deps-mjs': `1.0.0`,
},
},
{
pnpEnableEsmLoader: true,
},
async ({path, run, source}) => {
await run(`install`);

await xfs.writeFilePromise(ppath.join(path, `index.js`), `
require('no-deps-mjs/index.mjs')
console.log('ESM required')
`);

await expect(run(`node`, `index.js`)).resolves.toMatchObject({
code: 0,
stdout: `ESM required\n`,
});
},
),
);

test(
`it should throw ERR_MODULE_NOT_FOUND when statically importing a nonexistent file`,
makeTemporaryEnv(
Expand Down
2 changes: 1 addition & 1 deletion packages/yarnpkg-pnp/sources/hook.js

Large diffs are not rendered by default.

27 changes: 15 additions & 12 deletions packages/yarnpkg-pnp/sources/loader/applyPatch.ts
Original file line number Diff line number Diff line change
Expand Up @@ -275,20 +275,23 @@ export function applyPatch(pnpapi: PnpApi, opts: ApplyPatchOptions) {
return false;
};

// https://github.com/nodejs/node/blob/3743406b0a44e13de491c8590386a964dbe327bb/lib/internal/modules/cjs/loader.js#L1110-L1154
const originalExtensionJSFunction = Module._extensions[`.js`] as (module: NodeModule, filename: string) => void;
Module._extensions[`.js`] = function (module: NodeModule, filename: string) {
if (filename.endsWith(`.js`)) {
const pkg = nodeUtils.readPackageScope(filename);
if (pkg && pkg.data?.type === `module`) {
const err = nodeUtils.ERR_REQUIRE_ESM(filename, module.parent?.filename);
Error.captureStackTrace(err);
throw err;
// @ts-expect-error - Missing types
if (!process.features.require_module) {
// https://github.com/nodejs/node/blob/3743406b0a44e13de491c8590386a964dbe327bb/lib/internal/modules/cjs/loader.js#L1110-L1154
const originalExtensionJSFunction = Module._extensions[`.js`] as (module: NodeModule, filename: string) => void;
Module._extensions[`.js`] = function (module: NodeModule, filename: string) {
if (filename.endsWith(`.js`)) {
const pkg = nodeUtils.readPackageScope(filename);
if (pkg && pkg.data?.type === `module`) {
const err = nodeUtils.ERR_REQUIRE_ESM(filename, module.parent?.filename);
Error.captureStackTrace(err);
throw err;
}
}
}

originalExtensionJSFunction.call(this, module, filename);
};
originalExtensionJSFunction.call(this, module, filename);
};
}

const originalDlopen = process.dlopen;
process.dlopen = function (...args) {
Expand Down

0 comments on commit 3978649

Please sign in to comment.