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

fix(pnp): esm - support unflagged JSON modules #4786

Merged
merged 1 commit into from Aug 21, 2022
Merged
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
4 changes: 3 additions & 1 deletion .pnp.cjs

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

28 changes: 28 additions & 0 deletions .yarn/versions/736d10bb.yml
@@ -0,0 +1,28 @@
releases:
"@yarnpkg/cli": patch
"@yarnpkg/plugin-pnp": patch
"@yarnpkg/pnp": patch

declined:
- "@yarnpkg/esbuild-plugin-pnp"
- "@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"
1 change: 1 addition & 0 deletions CHANGELOG.md
Expand Up @@ -65,6 +65,7 @@ The following changes only affect people writing Yarn plugins:
### Compatibility

- Updates the PnP compatibility layer for TypeScript 4.8.1-rc
- The ESM loader now supports unflagged JSON modules.

## 3.2.2

Expand Down
4 changes: 3 additions & 1 deletion packages/acceptance-tests/pkg-tests-specs/package.json
Expand Up @@ -16,10 +16,12 @@
"@yarnpkg/fslib": "workspace:^",
"@yarnpkg/monorepo": "workspace:^",
"@yarnpkg/parsers": "workspace:^",
"@yarnpkg/pnp": "workspace:^",
"lodash": "^4.17.15",
"pkg-tests-core": "workspace:^",
"semver": "^7.1.2",
"tar": "^6.0.5"
"tar": "^6.0.5",
"tslib": "^2.4.0"
},
"engines": {
"node": ">=14.15.0"
Expand Down
54 changes: 52 additions & 2 deletions packages/acceptance-tests/pkg-tests-specs/sources/pnp-esm.test.ts
@@ -1,4 +1,5 @@
import {Filename, ppath, xfs} from '@yarnpkg/fslib';
import * as loaderFlags from '@yarnpkg/pnp/sources/esm-loader/loaderFlags';

describe(`Plug'n'Play - ESM`, () => {
test(
Expand Down Expand Up @@ -195,8 +196,8 @@ describe(`Plug'n'Play - ESM`, () => {
),
);

test(
`it should not resolve JSON files`,
(loaderFlags.HAS_UNFLAGGED_JSON_MODULES === false ? test : test.skip)(
`it should not resolve JSON modules without --experimental-json-modules`,
makeTemporaryEnv(
{
type: `module`,
Expand All @@ -218,6 +219,55 @@ describe(`Plug'n'Play - ESM`, () => {
),
);

(loaderFlags.HAS_UNFLAGGED_JSON_MODULES ? test : test.skip)(
`it should not resolve JSON modules without an import assertion`,
makeTemporaryEnv(
{
type: `module`,
},
async ({path, run, source}) => {
await expect(run(`install`)).resolves.toMatchObject({code: 0});

await xfs.writeFilePromise(
ppath.join(path, `index.js` as Filename),
`import './foo.json';`,
);
await xfs.writeFilePromise(ppath.join(path, `foo.json` as Filename), `{"name": "foo"}`);

await expect(run(`node`, `./index.js`)).rejects.toMatchObject({
code: 1,
stderr: expect.stringContaining(`ERR_IMPORT_ASSERTION_TYPE_MISSING`),
});
},
),
);

(loaderFlags.HAS_UNFLAGGED_JSON_MODULES ? test : test.skip)(
`it should resolve JSON modules with an import assertion`,
makeTemporaryEnv(
{
type: `module`,
},
async ({path, run, source}) => {
await expect(run(`install`)).resolves.toMatchObject({code: 0});

await xfs.writeFilePromise(
ppath.join(path, `index.js` as Filename),
`
import foo from './foo.json' assert { type: 'json' };
console.log(foo.name);
`,
);
await xfs.writeFilePromise(ppath.join(path, `foo.json` as Filename), `{"name": "foo"}`);

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

test(
`it should respect exports`,
makeTemporaryEnv(
Expand Down
2 changes: 1 addition & 1 deletion packages/yarnpkg-pnp/sources/esm-loader/built-loader.js

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

20 changes: 16 additions & 4 deletions packages/yarnpkg-pnp/sources/esm-loader/hooks/load.ts
@@ -1,12 +1,18 @@
import fs from 'fs';
import {fileURLToPath} from 'url';
import fs from 'fs';
import {fileURLToPath} from 'url';

import * as loaderUtils from '../loaderUtils';
import {HAS_JSON_IMPORT_ASSERTION_REQUIREMENT} from '../loaderFlags';
import * as loaderUtils from '../loaderUtils';

// The default `load` doesn't support reading from zip files
export async function load(
urlString: string,
context: { format: string | null | undefined },
context: {
format: string | null | undefined;
importAssertions?: {
type?: 'json';
};
},
nextLoad: typeof load,
): Promise<{ format: string, source: string, shortCircuit: boolean }> {
const url = loaderUtils.tryParseURL(urlString);
Expand All @@ -19,6 +25,12 @@ export async function load(
if (!format)
return nextLoad(urlString, context, nextLoad);

if (HAS_JSON_IMPORT_ASSERTION_REQUIREMENT && format === `json` && context.importAssertions?.type !== `json`) {
const err = new TypeError(`[ERR_IMPORT_ASSERTION_TYPE_MISSING]: Module "${urlString}" needs an import assertion of type "json"`) as TypeError & { code: string };
err.code = `ERR_IMPORT_ASSERTION_TYPE_MISSING`;
throw err;
}

return {
format,
source: await fs.promises.readFile(filePath, `utf8`),
Expand Down
12 changes: 4 additions & 8 deletions packages/yarnpkg-pnp/sources/esm-loader/loader.ts
Expand Up @@ -2,14 +2,10 @@ import {getFormat as getFormatHook} from './hooks/getFormat';
import {getSource as getSourceHook} from './hooks/getSource';
import {load as loadHook} from './hooks/load';
import {resolve as resolveHook} from './hooks/resolve';
import {HAS_CONSOLIDATED_HOOKS} from './loaderFlags';
import './fspatch';

const [major, minor] = process.versions.node.split(`.`).map(value => parseInt(value, 10));

// The hooks were consolidated in https://github.com/nodejs/node/pull/37468
const hasConsolidatedHooks = major > 16 || (major === 16 && minor >= 12);

export const resolve = resolveHook;
export const getFormat = hasConsolidatedHooks ? undefined : getFormatHook;
export const getSource = hasConsolidatedHooks ? undefined : getSourceHook;
export const load = hasConsolidatedHooks ? loadHook : undefined;
export const getFormat = HAS_CONSOLIDATED_HOOKS ? undefined : getFormatHook;
export const getSource = HAS_CONSOLIDATED_HOOKS ? undefined : getSourceHook;
export const load = HAS_CONSOLIDATED_HOOKS ? loadHook : undefined;
10 changes: 10 additions & 0 deletions packages/yarnpkg-pnp/sources/esm-loader/loaderFlags.ts
@@ -0,0 +1,10 @@
const [major, minor] = process.versions.node.split(`.`).map(value => parseInt(value, 10));

// The hooks were consolidated in https://github.com/nodejs/node/pull/37468
export const HAS_CONSOLIDATED_HOOKS = major > 16 || (major === 16 && minor >= 12);

// JSON modules were unflagged in https://github.com/nodejs/node/pull/41736
export const HAS_UNFLAGGED_JSON_MODULES = major > 17 || (major === 17 && minor >= 5) || (major === 16 && minor >= 15);

// JSON modules requires import assertions after https://github.com/nodejs/node/pull/40250
export const HAS_JSON_IMPORT_ASSERTION_REQUIREMENT = major > 17 || (major === 17 && minor >= 1) || (major === 16 && minor > 14);
15 changes: 10 additions & 5 deletions packages/yarnpkg-pnp/sources/esm-loader/loaderUtils.ts
@@ -1,9 +1,11 @@
import {NativePath} from '@yarnpkg/fslib';
import fs from 'fs';
import path from 'path';
import {URL} from 'url';
import {NativePath} from '@yarnpkg/fslib';
import fs from 'fs';
import path from 'path';
import {URL} from 'url';

import * as nodeUtils from '../loader/nodeUtils';
import * as nodeUtils from '../loader/nodeUtils';

import {HAS_UNFLAGGED_JSON_MODULES} from './loaderFlags';

export async function tryReadFile(path: NativePath): Promise<string | null> {
try {
Expand Down Expand Up @@ -48,6 +50,9 @@ export function getFileFormat(filepath: string): string | null {
);
}
case `.json`: {
if (HAS_UNFLAGGED_JSON_MODULES)
return `json`;

// TODO: Enable if --experimental-json-modules is present
// Waiting on https://github.com/nodejs/node/issues/36935
throw new Error(
Expand Down
2 changes: 2 additions & 0 deletions yarn.lock
Expand Up @@ -20249,10 +20249,12 @@ __metadata:
"@yarnpkg/fslib": "workspace:^"
"@yarnpkg/monorepo": "workspace:^"
"@yarnpkg/parsers": "workspace:^"
"@yarnpkg/pnp": "workspace:^"
lodash: "npm:^4.17.15"
pkg-tests-core: "workspace:^"
semver: "npm:^7.1.2"
tar: "npm:^6.0.5"
tslib: "npm:^2.4.0"
languageName: unknown
linkType: soft

Expand Down