Skip to content

Commit

Permalink
Injects the PnP runtime in the process before requiring the user conf…
Browse files Browse the repository at this point in the history
…ig (#5954)

**What's the problem this PR addresses?**

We added support for JS constraints thanks to the `yarn.config.cjs`
file. However this file is currently executed from within the Yarn
process, so it doesn't have access to the dependencies when operating
under the PnP installation mode.

Fixes #5878

**How did you fix it?**

I'm still on the fence on the right solution. This PR automatically
loads the `.pnp.cjs` file when the `loadUserConfig` file is called. An
alternative would be a post-install hook to allow each linker to inject
their runtime inside the process post-install, but that was more
involved and I figured it's worth more consideration.

Another alternative was to not do anything and expect users to setup the
PnP runtime themselves, but it feels something that Yarn should be able
to handle.

A third alternative would be to evaluate this file within a worker
thread, which would be started with the proper `execArgv` flags. It's
less intrusive than the post-install hook so I kinda like this idea, but
again - worth more consideration.

I'm working on enabling tests, and to this effect I'm checking whether
we can remove the `NODE_OPTIONS` values from the `makeTemporaryEnv`
subprocesses.

**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.
  • Loading branch information
arcanis authored Nov 13, 2023
1 parent c835804 commit f5b7abd
Show file tree
Hide file tree
Showing 4 changed files with 61 additions and 0 deletions.
34 changes: 34 additions & 0 deletions .yarn/versions/56678543.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 @@ -58,6 +58,8 @@ const mte = generatePkgDriver({
[`YARN_ENABLE_GLOBAL_CACHE`]: `false`,
// Older versions of Windows need this set to not have node throw an error
[`NODE_SKIP_PLATFORM_CHECK`]: `1`,
// We don't want the PnP runtime to be accidentally injected
[`NODE_OPTIONS`]: ``,
...rcEnv,
...env,
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,23 @@ describe(`Commands`, () => {
await expect(run(`constraints`)).rejects.toThrow(/This should fail/);
}));

it(`should allow requiring dependencies from the yarn.config.cjs file`, makeTemporaryEnv({
dependencies: {
[`no-deps`]: `1.0.0`,
},
}, async ({path, run, source}) => {
await run(`install`);

await writeFile(ppath.join(path, `yarn.config.cjs`), `
require('no-deps');
exports.constraints = ({Yarn}) => {
};
`);

await run(`constraints`);
}));

it(`shouldn't report errors when comparing identical objects`, makeTemporaryEnv({
foo: {
ok: true,
Expand Down
8 changes: 8 additions & 0 deletions packages/yarnpkg-core/sources/Project.ts
Original file line number Diff line number Diff line change
Expand Up @@ -711,6 +711,14 @@ export class Project {
}

async loadUserConfig() {
// TODO: We're injecting the .pnp.cjs in the environment, so that it's
// able to required the project dependencies. It's not ideal to hardcode
// this logic here, but I'm not quite sure yet what would be a better place.
//
const pnpPath = ppath.join(this.cwd, `.pnp.cjs`);
if (await xfs.existsPromise(pnpPath))
miscUtils.dynamicRequire(pnpPath).setup();

const configPath = ppath.join(this.cwd, `yarn.config.cjs`);
if (!await xfs.existsPromise(configPath))
return null;
Expand Down

0 comments on commit f5b7abd

Please sign in to comment.