Skip to content

Commit

Permalink
fix(pnp): esm - handle parentURL without a file: protocol (#5362)
Browse files Browse the repository at this point in the history
**What's the problem this PR addresses?**
When chaining Yarn PNP ESM loader with
[import-in-the-middle](https://github.com/DataDog/import-in-the-middle)
loader, INVALID_URL_SCHEME is thrown because `fileURLToPath()` is run on
a parent URL of `node:util?iitm=true` generated by the IITM loader.

```
2023-03-30T21:40:58.280Z 3364766 U TypeError [ERR_INVALID_URL_SCHEME]: The URL must be of scheme file
2023-03-30T21:40:58.280Z 3364766 U     at new NodeError (node:internal/errors:399:5)
2023-03-30T21:40:58.280Z 3364766 U     at fileURLToPath (node:internal/url:1212:11)
2023-03-30T21:40:58.280Z 3364766 U     at resolve$1 (file:///home/lizf/eve-roster/.pnp.loader.mjs:1993:77)
2023-03-30T21:40:58.280Z 3364766 U     at nextResolve (node:internal/modules/esm/hooks:654:28)
2023-03-30T21:40:58.280Z 3364766 U     at Hooks.resolve (node:internal/modules/esm/hooks:309:30)
2023-03-30T21:40:58.280Z 3364766 U     at ESMLoader.resolve (node:internal/modules/esm/loader:312:26)
2023-03-30T21:40:58.280Z 3364766 U     at ESMLoader.getModuleJob (node:internal/modules/esm/loader:172:38)
2023-03-30T21:40:58.281Z 3364766 U     at ModuleWrap.<anonymous> (node:internal/modules/esm/module_job:76:40)
2023-03-30T21:40:58.281Z 3364766 U     at link (node:internal/modules/esm/module_job:75:36) {
2023-03-30T21:40:58.281Z 3364766 U   code: 'ERR_INVALID_URL_SCHEME'
2023-03-30T21:40:58.281Z 3364766 U }
2023-03-30T21:40:58.281Z 3364766 U 
2023-03-30T21:40:58.281Z 3364766 U Node.js v19.8.1
```

**How did you fix it?**
Ensure fileURLToPath() is only run on file URLs by explicitly checking
the URL protocol; if it is not `file`, then default to CWD.

**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.

---------

Co-authored-by: merceyz <merceyz@users.noreply.github.com>
  • Loading branch information
lizthegrey and merceyz committed May 17, 2023
1 parent b7e27b4 commit 8b32064
Show file tree
Hide file tree
Showing 5 changed files with 90 additions and 5 deletions.
3 changes: 2 additions & 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/1c86bcd3.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"
61 changes: 59 additions & 2 deletions packages/acceptance-tests/pkg-tests-specs/sources/pnp-esm.test.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import {Filename, ppath, xfs} from '@yarnpkg/fslib';
import * as loaderFlags from '@yarnpkg/pnp/sources/esm-loader/loaderFlags';
import {Filename, npath, ppath, xfs} from '@yarnpkg/fslib';
import * as loaderFlags from '@yarnpkg/pnp/sources/esm-loader/loaderFlags';
import {pathToFileURL} from 'url';

describe(`Plug'n'Play - ESM`, () => {
test(
Expand Down Expand Up @@ -970,5 +971,61 @@ describe(`Plug'n'Play - ESM`, () => {
},
),
);

(loaderFlags.HAS_CONSOLIDATED_HOOKS ? test : test.skip)(
`it should allow importing files regardless of parent URL`,
makeTemporaryEnv(
{
type: `module`,
},
async ({path, run, source}) => {
await expect(run(`install`)).resolves.toMatchObject({code: 0});

await xfs.writeFilePromise(
ppath.join(path, `loader.js`),
`
export function resolve(specifier, context, next) {
if (specifier !== 'custom:foo') {
return next(specifier, context);
}
return {
shortCircuit: true,
url: 'custom:foo',
};
}
export function load(url, context, next) {
if (url !== 'custom:foo') {
return next(url, context);
}
return {
format: 'module',
source: "import { foo } from '${pathToFileURL(npath.fromPortablePath(ppath.join(path, `foo.js`)))}'\\nconsole.log(foo);",
shortCircuit: true,
};
}
`,
);

await xfs.writeFilePromise(
ppath.join(path, `foo.js`),
`export const foo = 42;`,
);

await xfs.writeFilePromise(
ppath.join(path, `index.js`),
`import 'custom:foo'`,
);

await expect(run(`node`, `--loader`, `./loader.js`, `./index.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.

2 changes: 1 addition & 1 deletion packages/yarnpkg-pnp/sources/esm-loader/hooks/resolve.ts
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ export async function resolve(

const {parentURL, conditions = []} = context;

const issuer = parentURL ? fileURLToPath(parentURL) : process.cwd();
const issuer = parentURL && loaderUtils.tryParseURL(parentURL)?.protocol === `file:` ? fileURLToPath(parentURL) : process.cwd();

// Get the pnpapi of either the issuer or the specifier.
// The latter is required when the specifier is an absolute path to a
Expand Down

0 comments on commit 8b32064

Please sign in to comment.