Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix!: call executePackageManagerRequest directly #430

Conversation

merceyz
Copy link
Member

@merceyz merceyz commented Mar 15, 2024

Calling executePackageManagerRequest directly instead of through Clipanion lets Node.js handle printing errors which results in the cause property being printed correctly.

Ref #458 and others like it.
Ref #425 (comment)

Depends on

Performance is roughly the same

$ hyperfine -w 5 "node ./dist-before/yarn.js -v" "node ./dist-after/yarn.js -v"
Benchmark 1: node ./dist-before/yarn.js -v
  Time (mean ± σ):     188.0 ms ±   9.9 ms    [User: 204.5 ms, System: 22.3 ms]
  Range (min … max):   179.0 ms … 222.1 ms    16 runs

Benchmark 2: node ./dist-after/yarn.js -v
  Time (mean ± σ):     186.5 ms ±   7.8 ms    [User: 202.5 ms, System: 22.4 ms]
  Range (min … max):   178.1 ms … 204.9 ms    16 runs

Summary
  node ./dist-after/yarn.js -v ran
    1.01 ± 0.07 times faster than node ./dist-before/yarn.js -v

BREAKING CHANGE:
Some errors are now printed to stderr instead of stdout

tests/main.test.ts Outdated Show resolved Hide resolved
tests/main.test.ts Outdated Show resolved Hide resolved
tests/main.test.ts Outdated Show resolved Hide resolved
@merceyz merceyz force-pushed the merceyz/perf/call-executePackageManagerRequest-directly branch from d1b5d0b to 0a0359e Compare March 15, 2024 22:16
@merceyz merceyz force-pushed the merceyz/perf/call-executePackageManagerRequest-directly branch from 0a0359e to ab77bb5 Compare April 20, 2024 12:48
merceyz and others added 2 commits April 20, 2024 14:50
Co-authored-by: Antoine du Hamel <duhamelantoine1995@gmail.com>
@merceyz merceyz force-pushed the merceyz/perf/call-executePackageManagerRequest-directly branch from ab77bb5 to 6e62f29 Compare April 20, 2024 12:51
@merceyz merceyz changed the title perf!: call executePackageManagerRequest directly fix!: call executePackageManagerRequest directly Apr 20, 2024
@merceyz merceyz marked this pull request as ready for review April 20, 2024 12:54
@merceyz merceyz force-pushed the merceyz/perf/call-executePackageManagerRequest-directly branch from dfddf92 to a82e453 Compare April 20, 2024 13:21
@merceyz merceyz merged commit 0f9b748 into nodejs:main Apr 20, 2024
10 checks passed
@merceyz merceyz deleted the merceyz/perf/call-executePackageManagerRequest-directly branch April 20, 2024 17:33
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.

2 participants