Skip to content

Commit

Permalink
fix(pnp): esm - return undefined source for commonjs (#5677)
Browse files Browse the repository at this point in the history
**What's the problem this PR addresses?**

The ESM loader always returns a source which after
nodejs/node#47999 landed causes stuff to break.

**How did you fix it?**

Return undefined source for commonjs modules

**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 Aug 23, 2023
1 parent 68039e0 commit 08f6f70
Show file tree
Hide file tree
Showing 5 changed files with 59 additions and 4 deletions.
2 changes: 1 addition & 1 deletion .pnp.loader.mjs

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/d70173f5.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"
28 changes: 28 additions & 0 deletions packages/acceptance-tests/pkg-tests-specs/sources/pnp-esm.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1004,4 +1004,32 @@ describe(`Plug'n'Play - ESM`, () => {
),
);
});

it(
`should use the commonjs resolver in commonjs files imported from ESM`,
makeTemporaryEnv(
{
type: `module`,
},
async ({path, run, source}) => {
await xfs.writeFilePromise(ppath.join(path, `foo.js`), `import './bar.cjs';`);
await xfs.writeFilePromise(
ppath.join(path, `bar.cjs`),
`
require('module')._extensions['.custom'] = require('module')._extensions['.js'];
require('./baz');
`,
);
await xfs.writeFilePromise(ppath.join(path, `baz.custom`), `console.log(42);`);

await expect(run(`install`)).resolves.toMatchObject({code: 0});

await expect(run(`node`, `./foo.js`)).resolves.toMatchObject({
code: 0,
stdout: `42\n`,
stderr: ``,
});
},
),
);
});
2 changes: 1 addition & 1 deletion packages/yarnpkg-pnp/sources/esm-loader/built-loader.js

Large diffs are not rendered by default.

4 changes: 2 additions & 2 deletions packages/yarnpkg-pnp/sources/esm-loader/hooks/load.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ export async function load(
};
},
nextLoad: typeof load,
): Promise<{ format: string, source: string, shortCircuit: boolean }> {
): Promise<{ format: string, source?: string, shortCircuit: boolean }> {
const url = loaderUtils.tryParseURL(urlString);
if (url?.protocol !== `file:`)
return nextLoad(urlString, context, nextLoad);
Expand Down Expand Up @@ -49,7 +49,7 @@ export async function load(

return {
format,
source: await fs.promises.readFile(filePath, `utf8`),
source: format === `commonjs` ? undefined : await fs.promises.readFile(filePath, `utf8`),
shortCircuit: true,
};
}

0 comments on commit 08f6f70

Please sign in to comment.