Skip to content

Conversation

@paul-soporan
Copy link
Member

@paul-soporan paul-soporan commented Jul 21, 2023

What's the problem this PR addresses?

Depends on #5595.

When yarnPath points to a different binary from the current one, Yarn forwards the command to the specified binary using child_process.execFileSync, which adds unnecessary overhead.

How did you fix it?

Made it use Module.runMain instead, the same trick we use in corepack starting from nodejs/corepack#97.

This speeds up every command when using Yarn from sources (or in other cases where corepack uses modern, and corepack and yarnPath point to different binaries).

yarn -v in such a situation:

  • Before: 0.263s
  • After: 0.229s
  • Improvement: 13%

Checklist

  • I have set the packages that need to be released for my changes to be effective.
  • I will check that all automated PR checks pass before the PR gets reviewed.

@paul-soporan paul-soporan marked this pull request as draft July 21, 2023 04:11
@paul-soporan paul-soporan marked this pull request as ready for review July 24, 2023 16:03
@merceyz
Copy link
Member

merceyz commented Jul 24, 2023

I haven't tested it but this might fix #3996.

@paul-soporan
Copy link
Member Author

🤔 Most likely, indeed.

@arcanis Can you please take a look at the v1 PR (yarnpkg/yarn#8793) so we can fully close that issue?

Copy link
Member

@merceyz merceyz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should document that

export async function runExit(argv: Array<string>, {cwd = ppath.cwd(), selfPath, pluginConfiguration}: {cwd: PortablePath, selfPath: PortablePath | null, pluginConfiguration: PluginConfiguration}) {
can resolve before Yarn is done running, make it not set the exit code when a different binary is ran, and rename it as the name wont be correct at that point.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants