Skip to content

Commit

Permalink
fix(pnp): esm - use correct error code when module not found (#5355)
Browse files Browse the repository at this point in the history
* fix(pnp): Use correct error code when module not found

* test(pnp): Add tests

* Commits file updates

* fix(pnp): re-throw regardless of what the error is

---------

Co-authored-by: Kristoffer K <[email protected]>
Co-authored-by: Maël Nison <[email protected]>
  • Loading branch information
3 people authored Apr 24, 2023
1 parent 03995ff commit 4927819
Show file tree
Hide file tree
Showing 5 changed files with 127 additions and 10 deletions.
15 changes: 11 additions & 4 deletions .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/cac399bc.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"
75 changes: 75 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 @@ -654,6 +654,81 @@ describe(`Plug'n'Play - ESM`, () => {
),
);

test(
`it should throw ERR_MODULE_NOT_FOUND when statically importing a nonexistent file`,
makeTemporaryEnv(
{
type: `module`,
},
async ({path, run, source}) => {
await expect(run(`install`)).resolves.toMatchObject({code: 0});

await xfs.writeFilePromise(ppath.join(path, `index.js`), `
import("./foo.js").catch((err) => {
console.log(err.code)
})
`);

await xfs.writeFilePromise(ppath.join(path, `foo.js`), `import './nonexistent.js'`);

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

test(
`it should throw ERR_MODULE_NOT_FOUND when dynamically importing a nonexistent file`,
makeTemporaryEnv(
{
type: `module`,
},
async ({path, run, source}) => {
await expect(run(`install`)).resolves.toMatchObject({code: 0});

await xfs.writeFilePromise(ppath.join(path, `index.js`), `
import("./nonexistent.js").catch((err) => {
console.log(err.code)
})
`);

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

test(
`it should throw ERR_PACKAGE_PATH_NOT_EXPORTED when subpath isn't exported`,
makeTemporaryEnv(
{
name: `foo`,
type: `module`,
exports: {
'./package.json': `./package.json`,
},
},
async ({path, run, source}) => {
await expect(run(`install`)).resolves.toMatchObject({code: 0});

await xfs.writeFilePromise(ppath.join(path, `index.mjs`), `
import('foo/bar').catch(err => {
console.log(err.code)
});
`);

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

// Tests /packages/yarnpkg-pnp/sources/esm-loader/fspatch.ts
test(
`it should support named exports in commonjs files`,
Expand Down
2 changes: 1 addition & 1 deletion packages/yarnpkg-pnp/sources/esm-loader/built-loader.js

Large diffs are not rendered by default.

18 changes: 13 additions & 5 deletions packages/yarnpkg-pnp/sources/esm-loader/hooks/resolve.ts
Original file line number Diff line number Diff line change
Expand Up @@ -100,11 +100,19 @@ export async function resolve(
}
}

const result = pnpapi.resolveRequest(specifier, issuer, {
conditions: new Set(conditions),
// TODO: Handle --experimental-specifier-resolution=node
extensions: allowLegacyResolve ? undefined : [],
});
let result;
try {
result = pnpapi.resolveRequest(specifier, issuer, {
conditions: new Set(conditions),
// TODO: Handle --experimental-specifier-resolution=node
extensions: allowLegacyResolve ? undefined : [],
});
} catch (err) {
if (err instanceof Error && `code` in err && err.code === `MODULE_NOT_FOUND`)
err.code = `ERR_MODULE_NOT_FOUND`;

throw err;
}

if (!result)
throw new Error(`Resolving '${specifier}' from '${issuer}' failed`);
Expand Down

0 comments on commit 4927819

Please sign in to comment.