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

feat: add an identifier for pnpmSyncPrepare #34

Merged
merged 2 commits into from
Jun 6, 2024
Merged

Conversation

g-chao
Copy link
Collaborator

@g-chao g-chao commented May 31, 2024

Summary

Support an optional parameter lockfileId when calling the pnpmSyncPrepareAsync API.
The purpose of this parameter is let pnpm-sync to recognize the entries in the .pnpm-config.json file, so that the pnpm-sync can update these entries correctly, also be able to remove invalid entires

Details

When calling the pnpmSyncPrepareAsync API, how does pnpm-sync know which entries need to be update in the .pnpm-config.json? Especially in a large Monorepo with Rush subspace feature enabled where there could have tens of pnpm-lock.yaml files.

Previously, when generating the .pnpm-config.json, pnpm-sync read the existing entries, only append new unique entires there. This is to ensure the tool doesn't delete entries where could belong to other pnpm-lock.yaml. But, it has a drawback where, since it does not delete any entries, it is possible, in your local development, there could be some invalid entries in the .pnpm-config.json due to frequently pnpm-lock.yaml changes. It might cause issue as time goes by, and when it happens, you will need to manually delete .pnpm-config.json file or node_modules to regenerate them. It brings inconvenience.

In this PR, we add a optional parameter identifier when calling the pnpmSyncPrepareAsync API. So that, the pnpm-sync could recognize the pnpm-lock.yaml file it processed, then it able to delete the invalid entries in the .pnpm-config.json to avoid unnecessary or invalid sync actions in the later steps.

* An identifier that can be used to recognize the `pnpm-lock.yaml`
*/
identifier?: string;

Copy link
Member

@octogonz octogonz Jun 6, 2024

Choose a reason for hiding this comment

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

@g-chao I think it would also be useful to provide a way to delete unknown identifiers, for example if a Rush subspace was deleted.

The setting could be like:

/** 
  * The identifier to be prepared by this operation.  If it an entry already exists with this name,
  * then it will be replaced.
  */
"identifier": "subspace2",

/** 
 * Instruct the API to delete any identifiers that don't appear in this list.
 */
"pruneIdentifiersExcept": [ "subspace1", "subspace2", "subspace3" ]

Since my idea here is basically a separate feature, maybe it could be introduced later as a separate PR. Along with a corresponding CLI like:

pnpm-sync prepare --prune-identifiers-except subspace1,subspace2,subspace3

or

pnpm-sync prune --except subspace1,subspace2,subspace3

Copy link
Collaborator Author

@g-chao g-chao Jun 6, 2024

Choose a reason for hiding this comment

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

We need to pass pnpm-lock.yaml to pnpm-sync prepare. So in the below example won't work, unless we pass ALL pnpm-lock.yaml files to the command, which is not intended.

pnpm-sync prepare --prune-identifiers-except subspace1,subspace2,subspace3

In the situation that Rush subspace was deleted, the identifiers related to this subspace will be deleted as well along the regular rush install/update events

@g-chao g-chao merged commit b01b345 into main Jun 6, 2024
2 checks passed
@g-chao g-chao deleted the chao/add-identifier branch June 6, 2024 23:52
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