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

[rush] Add support for pnpm ignoredOptionalDependencies #4928

Merged

Conversation

glitchkyle
Copy link
Contributor

@glitchkyle glitchkyle commented Sep 18, 2024

Summary

Fixes #4859

Changes include updating pnpm configuration to use pnpm's ignoredOptionalDependencies setting. End-user should now be able to specify optional dependencies in common/config/rush/pnpm-config.json in case some optional dependencies are required at a later point.

Details

Changes focused on updating the rush init template and havingignoredOptionalDependencies in common/temp/package.json copied from common/config/rush/pnpm-config.json. Changes might not fully reflect until schema changes and rushVersion are updated after merging this PR.

How it was tested

  1. Ran rush init (when I had the schema changed in common/config/rush/pnpm-config.json) to check if common/config/rush/pnpm-config.json contained newly added field globalIgnoredOptionalDependencies
  2. Ran rush update with local build after adding fsevents in globalIgnoredOptionalDependencies to see if fsevents was in common/temp/package.json

Impacted documentation

PR might affect pnpm-config.json schema (https://rushjs.io/pages/configs/pnpm-config_json/)

@glitchkyle glitchkyle changed the title [Rush] Add support for pnpm ignoredOptionalDependencies [rush] Add support for pnpm ignoredOptionalDependencies Sep 18, 2024
@glitchkyle
Copy link
Contributor Author

@microsoft-github-policy-service agree

@glitchkyle
Copy link
Contributor Author

Making a draft PR for now while waiting for response on Zullipchat regarding testing changes and how to fix failing CI.

@glitchkyle glitchkyle force-pushed the support-pnpm-ignore-optional-dependencies branch 3 times, most recently from 8ff0228 to a9e4ede Compare October 13, 2024 03:10
@glitchkyle glitchkyle force-pushed the support-pnpm-ignore-optional-dependencies branch from a9e4ede to 8ef00ef Compare October 17, 2024 01:20
@glitchkyle glitchkyle marked this pull request as ready for review October 17, 2024 01:37
Copy link
Member

@D4N14L D4N14L left a comment

Choose a reason for hiding this comment

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

@glitchkyle it looks like your description mentions updating the rush init template, but I don't see any changes relating to that. Were they missed?

@glitchkyle
Copy link
Contributor Author

@glitchkyle it looks like your description mentions updating the rush init template, but I don't see any changes relating to that. Were they missed?

I could be wrong here but I thought the pnpm-config.json that contains the rush init template will be changed once the rushVersion is also updated.

Should be a quick change though by adding globalIgnoredOptionalDependencies in libraries/rush-lib/assets/rush-init/common/config/rush/pnpm-config.json.

@D4N14L
Copy link
Member

D4N14L commented Oct 18, 2024

@glitchkyle it looks like your description mentions updating the rush init template, but I don't see any changes relating to that. Were they missed?

I could be wrong here but I thought the pnpm-config.json that contains the rush init template will be changed once the rushVersion is also updated.

Should be a quick change though by adding globalIgnoredOptionalDependencies in libraries/rush-lib/assets/rush-init/common/config/rush/pnpm-config.json.

@glitchkyle yes you'll need to manually update the template. It's not updated as a part of any auto-generation process.... though that would be nice.

@D4N14L D4N14L merged commit 536cb53 into microsoft:main Oct 18, 2024
4 checks passed
@glitchkyle glitchkyle deleted the support-pnpm-ignore-optional-dependencies branch November 18, 2024 00:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Closed
Development

Successfully merging this pull request may close these issues.

[rush] feature request: support pnpm's ignoredOptionalDependencies setting in pnpm-config.json
2 participants