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

feat(pnp): enable ESM support by default #5574

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
23 changes: 23 additions & 0 deletions .yarn/versions/380139a7.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
releases:
"@yarnpkg/cli": minor
"@yarnpkg/plugin-pnp": minor

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"
2 changes: 0 additions & 2 deletions .yarnrc.yml
Original file line number Diff line number Diff line change
Expand Up @@ -69,8 +69,6 @@ packageExtensions:
typedoc-plugin-yarn:
optional: true

pnpEnableEsmLoader: true

preferInteractive: true

supportedArchitectures:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ describe(`Commands`, () => {
await expect(run(`stage`, `-n`, {cwd: path})).resolves.toMatchObject({
stdout: [
`${npath.fromPortablePath(`${path}/.pnp.cjs`)}\n`,
`${npath.fromPortablePath(`${path}/.pnp.loader.mjs`)}\n`,
`${npath.fromPortablePath(`${path}/.yarn/global/metadata/npm/3fb1ad/localhost/no-deps.json`)}\n`,
`${npath.fromPortablePath(`${path}/.yarn/global/cache/no-deps-npm-1.0.0-cf533b267a-0c0.zip`)}\n`,
`${npath.fromPortablePath(`${path}/.yarn/cache/.gitignore`)}\n`,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,8 +44,9 @@ exports[`Features Merge Conflict Resolution it should properly fix merge conflic
➤ YN0013: │ No new packages added to the cache, but one was removed (- 0.78 KiB).
➤ YN0000: └ Completed
➤ YN0000: ┌ Link step
➤ YN0000: │ ESM support for PnP uses the experimental loader API and is therefore experimental
➤ YN0000: └ Completed
➤ YN0000: · Done
➤ YN0000: · Done with warnings
",
}
`;
Expand Down Expand Up @@ -144,8 +145,9 @@ exports[`Features Merge Conflict Resolution it should support fixing cherry-pick
➤ YN0013: │ No new packages added to the cache, but one was removed (- 0.78 KiB).
➤ YN0000: └ Completed
➤ YN0000: ┌ Link step
➤ YN0000: │ ESM support for PnP uses the experimental loader API and is therefore experimental
➤ YN0000: └ Completed
➤ YN0000: · Done
➤ YN0000: · Done with warnings
",
}
`;
Expand Down Expand Up @@ -194,8 +196,9 @@ exports[`Features Merge Conflict Resolution it should support fixing rebase conf
➤ YN0013: │ No new packages added to the cache, but one was removed (- 0.78 KiB).
➤ YN0000: └ Completed
➤ YN0000: ┌ Link step
➤ YN0000: │ ESM support for PnP uses the experimental loader API and is therefore experimental
➤ YN0000: └ Completed
➤ YN0000: · Done
➤ YN0000: · Done with warnings
",
}
`;
Expand Down Expand Up @@ -244,8 +247,9 @@ exports[`Features Merge Conflict Resolution it shouldn't re-fetch the lockfile m
➤ YN0013: │ No new packages added to the cache, but one was removed (- 0.78 KiB).
➤ YN0000: └ Completed
➤ YN0000: ┌ Link step
➤ YN0000: │ ESM support for PnP uses the experimental loader API and is therefore experimental
➤ YN0000: └ Completed
➤ YN0000: · Done
➤ YN0000: · Done with warnings
",
}
`;
Original file line number Diff line number Diff line change
Expand Up @@ -38,9 +38,7 @@ describe(`Install Artifact Cleanup`, () => {
await expect(xfs.existsPromise(`${path}/.pnp.data.json` as PortablePath)).resolves.toStrictEqual(false);
}));

it(`should remove the .pnp.loader.mjs file after switching to the ${linker} linker`, makeTemporaryEnv({}, {
pnpEnableEsmLoader: true,
}, async ({path, run, source}) => {
it(`should remove the .pnp.loader.mjs file after switching to the ${linker} linker`, makeTemporaryEnv({}, async ({path, run, source}) => {
await run(`install`);

// Sanity check
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@ describe(`Features`, () => {
dependenciesMeta: {
[`no-deps-scripted`]: {built: false},
},
}, {
pnpEnableEsmLoader: false,
}, async ({path, run, source}) => {
let {stdout} = await run(`install`);
expect(stdout).toMatch(/lists build scripts/); // sanity check
Expand Down Expand Up @@ -69,6 +71,8 @@ describe(`Features`, () => {
dependenciesMeta: {
[`no-deps-scripted`]: {built: false},
},
}, {
pnpEnableEsmLoader: false,
}, async ({path, run, source}) => {
let {stdout} = await run(`install`);
expect(stdout).toMatch(/lists build scripts/); // sanity check
Expand Down Expand Up @@ -116,6 +120,8 @@ describe(`Features`, () => {
dependenciesMeta: {
[`no-deps-scripted`]: {built: false},
},
}, {
pnpEnableEsmLoader: false,
}, async ({path, run, source}) => {
let {stdout} = await run(`install`, {env: {FORCE_COLOR: `1`}});
expect(stdout).toMatch(/lists build scripts/); // sanity check
Expand Down Expand Up @@ -143,6 +149,8 @@ describe(`Features`, () => {
dependenciesMeta: {
[`no-deps-scripted`]: {built: false},
},
}, {
pnpEnableEsmLoader: false,
}, async ({path, run, source}) => {
let {stdout} = await run(`install`);
expect(stdout).toMatch(/lists build scripts/); // sanity check
Expand Down
36 changes: 0 additions & 36 deletions packages/acceptance-tests/pkg-tests-specs/sources/pnp-esm.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -120,9 +120,6 @@ describe(`Plug'n'Play - ESM`, () => {
`it should allow relative imports with search params`,
makeTemporaryEnv(
{},
{
pnpEnableEsmLoader: true,
},
async ({path, run, source}) => {
await expect(run(`install`)).resolves.toMatchObject({code: 0});

Expand All @@ -142,9 +139,6 @@ describe(`Plug'n'Play - ESM`, () => {
`it should preserve search params in the resolve result (cache busting)`,
makeTemporaryEnv(
{},
{
pnpEnableEsmLoader: true,
},
async ({path, run, source}) => {
await expect(run(`install`)).resolves.toMatchObject({code: 0});

Expand Down Expand Up @@ -173,9 +167,6 @@ describe(`Plug'n'Play - ESM`, () => {
`it should allow absolute imports with search params`,
makeTemporaryEnv(
{},
{
pnpEnableEsmLoader: true,
},
async ({path, run, source}) => {
await expect(run(`install`)).resolves.toMatchObject({code: 0});

Expand Down Expand Up @@ -361,9 +352,6 @@ describe(`Plug'n'Play - ESM`, () => {
`it should load extensionless commonjs files as an entrypoint`,
makeTemporaryEnv(
{ },
{
pnpEnableEsmLoader: true,
},
async ({path, run, source}) => {
await xfs.writeFilePromise(ppath.join(path, `index`), `console.log(typeof require === 'undefined')`);

Expand All @@ -381,9 +369,6 @@ describe(`Plug'n'Play - ESM`, () => {
`it should load symlinked extensionless commonjs files as an entrypoint`,
makeTemporaryEnv(
{ },
{
pnpEnableEsmLoader: true,
},
async ({path, run, source}) => {
await xfs.mkdirPromise(ppath.join(path, `lib`));
await xfs.writeFilePromise(ppath.join(path, `lib/index`), `console.log(typeof require === 'undefined')`);
Expand All @@ -403,9 +388,6 @@ describe(`Plug'n'Play - ESM`, () => {
`it should not allow extensionless commonjs imports`,
makeTemporaryEnv(
{ },
{
pnpEnableEsmLoader: true,
},
async ({path, run, source}) => {
await xfs.writeFilePromise(ppath.join(path, `index.mjs`), `import bin from './cjs-bin';\nconsole.log(bin)`);
await xfs.writeFilePromise(ppath.join(path, `cjs-bin`), `module.exports = {foo: 'bar'}`);
Expand All @@ -426,9 +408,6 @@ describe(`Plug'n'Play - ESM`, () => {
{
type: `module`,
},
{
pnpEnableEsmLoader: true,
},
async ({path, run, source}) => {
await xfs.writeFilePromise(ppath.join(path, `index`), ``);

Expand Down Expand Up @@ -505,9 +484,6 @@ describe(`Plug'n'Play - ESM`, () => {
"is-number": `1.0.0`,
},
},
{
pnpEnableEsmLoader: true,
},
async ({path, run, source}) => {
await xfs.writeFilePromise(ppath.join(path, `index.js`), `require('no-deps');\nimport('is-number').then(() => console.log(42))`);

Expand All @@ -525,9 +501,6 @@ describe(`Plug'n'Play - ESM`, () => {
`it should set the main module`,
makeTemporaryEnv(
{},
{
pnpEnableEsmLoader: true,
},
async ({path, run, source}) => {
await expect(run(`install`)).resolves.toMatchObject({code: 0});

Expand All @@ -554,9 +527,6 @@ describe(`Plug'n'Play - ESM`, () => {
`it should suppress the experimental ESM loader warning`,
makeTemporaryEnv(
{},
{
pnpEnableEsmLoader: true,
},
async ({path, run, source}) => {
await expect(run(`install`)).resolves.toMatchObject({code: 0});

Expand All @@ -579,9 +549,6 @@ describe(`Plug'n'Play - ESM`, () => {
'no-deps-esm': `1.0.0`,
},
},
{
pnpEnableEsmLoader: true,
},
async ({path, run, source}) => {
await expect(run(`install`)).resolves.toMatchObject({code: 0});

Expand Down Expand Up @@ -609,9 +576,6 @@ describe(`Plug'n'Play - ESM`, () => {
'no-deps-mjs': `1.0.0`,
},
},
{
pnpEnableEsmLoader: true,
},
async ({path, run, source}) => {
await expect(run(`install`)).resolves.toMatchObject({code: 0});

Expand Down
17 changes: 14 additions & 3 deletions packages/gatsby/content/features/plugnplay.md
Original file line number Diff line number Diff line change
Expand Up @@ -74,11 +74,22 @@ yarn node ./server.js
require('./.pnp.cjs').setup();
```

As a quick tip, all `yarn node` typically does is set the `NODE_OPTIONS` environment variable to use the [`--require`](https://nodejs.org/api/cli.html#cli_r_require_module) option from Node, associated with the path of the `.pnp.cjs` file. You can easily apply this operation yourself if you prefer:
As a quick tip, all `yarn node` typically does is set the `NODE_OPTIONS` environment variable to use the [`--require`](https://nodejs.org/api/cli.html#cli_r_require_module) and [`--experimental-loader`](https://nodejs.org/api/cli.html#--experimental-loadermodule) option to setup PnP support. You can apply this operation yourself if you prefer.

```
If you don't need ESM support you can use the following:

```sh
node -r ./.pnp.cjs ./server.js
NODE_OPTIONS="--require $(pwd)/.pnp.cjs" node ./server.js
# or
NODE_OPTIONS="--require \"$(pwd)/.pnp.cjs\"" node ./server.js
```

If you need ESM support you can use the following:

```sh
node -r ./.pnp.cjs --experimental-loader ./.pnp.loader.mjs ./server.js
# or
NODE_OPTIONS="--require \"$(pwd)/.pnp.cjs\" --experimental-loader \"$(pwd)/.pnp.loader.mjs\"" node ./server.js
```

## PnP `loose` mode
Expand Down
4 changes: 2 additions & 2 deletions packages/gatsby/static/configuration/yarnrc.json
Original file line number Diff line number Diff line change
Expand Up @@ -621,9 +621,9 @@
},
"pnpEnableEsmLoader": {
"_package": "@yarnpkg/plugin-pnp",
"description": "If true, Yarn will generate an experimental ESM loader (`.pnp.loader.mjs`). Yarn tries to automatically detect whether ESM support is required.",
"description": "If true, Yarn will generate an experimental ESM loader (`.pnp.loader.mjs`).",
"type": "boolean",
"default": false
"default": true
},
"pnpEnableInlining": {
"_package": "@yarnpkg/plugin-pnp",
Expand Down
26 changes: 2 additions & 24 deletions packages/plugin-pnp/sources/PnpLinker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -98,8 +98,6 @@ export class PnpInstaller implements Installer {
location: PortablePath;
}> = new Map();

private isESMLoaderRequired: boolean = false;

constructor(protected opts: LinkOptions) {
this.opts = opts;
}
Expand Down Expand Up @@ -153,9 +151,6 @@ export class PnpInstaller implements Installer {
}
}

if (customPackageData.manifest.type === `module`)
this.isESMLoaderRequired = true;

dependencyMeta = this.opts.project.getDependencyMeta(devirtualizedLocator, pkg.version);
}

Expand Down Expand Up @@ -236,7 +231,7 @@ export class PnpInstaller implements Installer {

const pnpPath = getPnpPath(this.opts.project);

if (!this.isEsmEnabled())
if (!this.opts.project.configuration.get(`pnpEnableEsmLoader`))
await xfs.removePromise(pnpPath.esmLoader);

if (this.opts.project.configuration.get(`nodeLinker`) !== `pnp`) {
Expand Down Expand Up @@ -298,22 +293,6 @@ export class PnpInstaller implements Installer {
// Nothing to transform
}

private isEsmEnabled() {
if (this.opts.project.configuration.sources.has(`pnpEnableEsmLoader`))
return this.opts.project.configuration.get(`pnpEnableEsmLoader`);

if (this.isESMLoaderRequired)
return true;

for (const workspace of this.opts.project.workspaces) {
if (workspace.manifest.type === `module`) {
return true;
}
}

return false;
}

async finalizeInstallWithPnp(pnpSettings: PnpSettings) {
const pnpPath = getPnpPath(this.opts.project);

Expand Down Expand Up @@ -350,7 +329,7 @@ export class PnpInstaller implements Installer {
});
}

if (this.isEsmEnabled()) {
if (this.opts.project.configuration.get(`pnpEnableEsmLoader`)) {
this.opts.report.reportWarning(MessageName.UNNAMED, `ESM support for PnP uses the experimental loader API and is therefore experimental`);
await xfs.changeFilePromise(pnpPath.esmLoader, getESMLoaderTemplate(), {
automaticNewlines: true,
Expand Down Expand Up @@ -509,7 +488,6 @@ async function extractCustomPackageData(fetchResult: FetchResult) {
manifest: {
scripts: manifest.scripts,
preferUnplugged: manifest.preferUnplugged,
type: manifest.type,
},
misc: {
extractHint: jsInstallUtils.getExtractHint(fetchResult),
Expand Down
4 changes: 2 additions & 2 deletions packages/plugin-pnp/sources/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -114,9 +114,9 @@ const plugin: Plugin<CoreHooks & StageHooks> = {
isArray: true,
},
pnpEnableEsmLoader: {
description: `If true, Yarn will generate an ESM loader (\`.pnp.loader.mjs\`). If this is not explicitly set Yarn tries to automatically detect whether ESM support is required.`,
description: `If true, Yarn will generate an ESM loader (\`.pnp.loader.mjs\`).`,
type: SettingsType.BOOLEAN,
default: false,
default: true,
},
pnpEnableInlining: {
description: `If true, the PnP data will be inlined along with the generated loader`,
Expand Down