Skip to content

Commit

Permalink
fix(core): determine if a module is a builtin using module.isBuiltin (
Browse files Browse the repository at this point in the history
#5997)

**What's the problem this PR addresses?**
<!-- Describe the rationale of your PR. -->
`builder plugin` may output `node:process` instead of `process` in the
generated file.
When executing `yarn.`, it will report an error like:
```
This plugin cannot access the package referenced via node:process which is neither a builtin, nor an exposed entry
```

** Environment
```
❯ yarn tsc --version
Version 5.3.2
```
<!-- Link all issues that it closes. (Closes/Resolves #xxxx.) -->

...

**How did you fix it?**
<!-- A detailed description of your implementation. -->

**Checklist**
<!--- Don't worry if you miss something, chores are automatically
tested. -->
<!--- This checklist exists to help you remember doing the chores when
you submit a PR. -->
<!--- Put an `x` in all the boxes that apply. -->
- [x] I have read the [Contributing
Guide](https://yarnpkg.com/advanced/contributing).

<!-- See
https://yarnpkg.com/advanced/contributing#preparing-your-pr-to-be-released
for more details. -->
<!-- Check with `yarn version check` and fix with `yarn version check
-i` -->
- [x] I have set the packages that need to be released for my changes to
be effective.

<!-- The "Testing chores" workflow validates that your PR follows our
guidelines. -->
<!-- If it doesn't pass, click on it to see details as to what your PR
might be missing. -->
- [x] I will check that all automated PR checks pass before the PR gets
reviewed.

---------

Co-authored-by: Maël Nison <[email protected]>
Co-authored-by: Kristoffer K. <[email protected]>
  • Loading branch information
3 people authored Aug 25, 2024
1 parent cadd19e commit 3b156c9
Show file tree
Hide file tree
Showing 3 changed files with 61 additions and 3 deletions.
34 changes: 34 additions & 0 deletions .yarn/versions/44a33643.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
releases:
"@yarnpkg/cli": patch
"@yarnpkg/core": patch

declined:
- "@yarnpkg/plugin-compat"
- "@yarnpkg/plugin-constraints"
- "@yarnpkg/plugin-dlx"
- "@yarnpkg/plugin-essentials"
- "@yarnpkg/plugin-exec"
- "@yarnpkg/plugin-file"
- "@yarnpkg/plugin-git"
- "@yarnpkg/plugin-github"
- "@yarnpkg/plugin-http"
- "@yarnpkg/plugin-init"
- "@yarnpkg/plugin-interactive-tools"
- "@yarnpkg/plugin-link"
- "@yarnpkg/plugin-nm"
- "@yarnpkg/plugin-npm"
- "@yarnpkg/plugin-npm-cli"
- "@yarnpkg/plugin-pack"
- "@yarnpkg/plugin-patch"
- "@yarnpkg/plugin-pnp"
- "@yarnpkg/plugin-pnpm"
- "@yarnpkg/plugin-stage"
- "@yarnpkg/plugin-typescript"
- "@yarnpkg/plugin-version"
- "@yarnpkg/plugin-workspace-tools"
- "@yarnpkg/builder"
- "@yarnpkg/doctor"
- "@yarnpkg/extensions"
- "@yarnpkg/nm"
- "@yarnpkg/pnpify"
- "@yarnpkg/sdks"
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import {mockPluginServer} from './plugins.utility';
const COMMANDS_PLUGIN = (name: string, {async = false, printOnBoot = false, thirdParty = false} = {}) => `
const factory = ${async ? `async` : ``} r => {
const {Command} = r('clipanion');
const path = r('node:path');
if (${printOnBoot})
console.log('Booting ${name.toUpperCase()}');
Expand All @@ -22,6 +23,14 @@ const factory = ${async ? `async` : ``} r => {
this.context.stdout.write('Executing ${name.toUpperCase()}\\n');
}
},
class MyCommandPath extends Command {
static paths = [['${name}', 'path']];
async execute() {
this.context.stdout.write(path.posix.join('a', 'b') + '\\n');
}
},
],
},
};
Expand Down Expand Up @@ -77,6 +86,19 @@ describe(`Features`, () => {
});
}));

test(`it should support plugins using builtin modules`, makeTemporaryEnv({
}, async ({path, run, source}) => {
await xfs.writeFilePromise(`${path}/plugin-a.js` as PortablePath, COMMANDS_PLUGIN(`a`));

await xfs.writeFilePromise(`${path}/.yarnrc.yml` as PortablePath, stringifySyml({
plugins: [`./plugin-a.js`],
}));

await expect(run(`a`, `path`)).resolves.toMatchObject({
stdout: `a/b\n`,
});
}));

test(`it should accept asynchronous plugins`, makeTemporaryEnv({
}, async ({path, run, source}) => {
await xfs.writeFilePromise(`${path}/plugin-a.js` as PortablePath, COMMANDS_PLUGIN(`a`, {async: true}));
Expand Down
8 changes: 5 additions & 3 deletions packages/yarnpkg-core/sources/Configuration.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import camelcase
import {isCI, isPR, GITHUB_ACTIONS} from 'ci-info';
import {UsageError} from 'clipanion';
import {parse as parseDotEnv} from 'dotenv';
import {builtinModules} from 'module';
import {isBuiltin} from 'module';
import pLimit, {Limit} from 'p-limit';
import {PassThrough, Writable} from 'stream';
import {WriteStream} from 'tty';
Expand Down Expand Up @@ -1265,8 +1265,7 @@ export class Configuration {
const thirdPartyPlugins = new Map<string, Plugin>([]);
if (pluginConfiguration !== null) {
const requireEntries = new Map();
for (const request of builtinModules)
requireEntries.set(request, () => miscUtils.dynamicRequire(request));

for (const [request, embedModule] of pluginConfiguration.modules)
requireEntries.set(request, () => embedModule);

Expand All @@ -1284,6 +1283,9 @@ export class Configuration {

const pluginRequireEntries = new Map(requireEntries);
const pluginRequire = (request: string) => {
if (isBuiltin(request))
return miscUtils.dynamicRequire(request);

if (pluginRequireEntries.has(request)) {
return pluginRequireEntries.get(request)();
} else {
Expand Down

0 comments on commit 3b156c9

Please sign in to comment.