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(core-utils): trim spawnPackageManager output #3866

Merged
merged 4 commits into from
Mar 3, 2025

Conversation

rahulptl165
Copy link
Contributor

Fixes #3864 by @mato533

  • I have read the contribution documentation for this project.
  • I agree to follow the code of conduct that this project follows, as appropriate.
  • The changes are appropriately documented (if applicable).
  • The changes have sufficient test coverage (if applicable).
  • The testsuite passes successfully on my local machine (if applicable).

Summarize your changes:

  • Added .trim() to pnpm config get outputs:
  • Modified the retrieval of hoistPattern, publicHoistPattern, and nodeLinker to use .trim() on the results of spawnPackageManager. This removes trailing newlines (\n) from the pnpm config get output, ensuring accurate string comparisons.
  • Before: "undefined\n" !== 'undefined' caused the check to misinterpret unset values as custom configurations.
  • After: "undefined\n".trim() becomes "undefined", correctly triggering the node-linker check when hoist patterns are unset.

Sorry, something went wrong.

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
@rahulptl165 rahulptl165 requested a review from a team as a code owner February 28, 2025 09:16
@erikian erikian changed the title Issue#3864 fix(core-utils): trim spawnPackageManager output Feb 28, 2025
Copy link
Member

@erikian erikian left a comment

Choose a reason for hiding this comment

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

Thanks for the contribution! spawnPackageManager is also used in a few other places, we should probably do the trimming directly in spawnPackageManager before returning the result, so that all consumers get trimmed strings. Could you make that change?

@rahulptl165
Copy link
Contributor Author

Yes sir !

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
@rahulptl165
Copy link
Contributor Author

Hi @erikian, thanks for your feedback. i have implemented the requested changes.

Copy link
Member

@erikian erikian left a comment

Choose a reason for hiding this comment

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

LGTM! Just one minor suggestion.

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
@rahulptl165
Copy link
Contributor Author

Implemented the suggested changes.

@erickzhao erickzhao added this pull request to the merge queue Mar 3, 2025
Merged via the queue into electron:main with commit 632117e Mar 3, 2025
12 checks passed
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.

check system for pnpm is not work
4 participants