Skip to content

feat(find-workspace-packages): accept cached workspaceManifest option#8120

Closed
gluxon wants to merge 2 commits intopnpm:mainfrom
gluxon:cached-workspaceManifest-option
Closed

feat(find-workspace-packages): accept cached workspaceManifest option#8120
gluxon wants to merge 2 commits intopnpm:mainfrom
gluxon:cached-workspaceManifest-option

Conversation

@gluxon
Copy link
Copy Markdown
Member

@gluxon gluxon commented May 23, 2024

Context

This is a change for #8020 (comment). My full plan is:

  1. Read the pnpm-workspace.yaml in @pnpm/config as suggested here
  2. Pass the in-memory workspaceManifest to find-workspace-packages as an optional argument.

This PR allows 2 to be done in a follow-up PR:

Changes

This PR adds a new workspaceManifest option. A test was added, but this new option is not yet used by other parts of pnpm.

Alternative

Alternatively we could "break" the find-workspace-packages utilities and force users of the @pnpm/workspace.find-packages to supply pnpm-workspace.yaml's contents.

One API change we could make is:

 export async function findWorkspacePackages (
   workspaceRoot: string,
+   patterns: string[] | WorkspaceManifest | undefined,
   opts?: FindWorkspacePackagesOptions
 ): Promise<Project[]>;

Or alternatively the WorkspaceManifest wouldn't be passed at all. Users of @pnpm/workspace.find-packages would pass workspaceManifest.packages as the patterns argument.

 export async function findWorkspacePackages (
   workspaceRoot: string,
+   patterns: string[],
   opts?: FindWorkspacePackagesOptions
 ): Promise<Project[]>;

Let me know if either of these alternatives are preferred.

@gluxon
Copy link
Copy Markdown
Member Author

gluxon commented May 23, 2024

Since this is adding new optional behavior, I think it can merge into pnpm's main branch. But let me know if I should target the catalogs feature branch instead.

@gluxon gluxon marked this pull request as ready for review May 23, 2024 19:28
@gluxon gluxon requested a review from zkochan as a code owner May 23, 2024 19:28
@gluxon gluxon closed this May 25, 2024
@gluxon gluxon deleted the cached-workspaceManifest-option branch May 25, 2024 04:46
@gluxon gluxon restored the cached-workspaceManifest-option branch May 25, 2024 04:46
@gluxon gluxon deleted the cached-workspaceManifest-option branch May 25, 2024 04:47
@gluxon gluxon restored the cached-workspaceManifest-option branch May 31, 2024 21:14
@gluxon gluxon reopened this May 31, 2024
@gluxon gluxon force-pushed the cached-workspaceManifest-option branch from 5a26cf1 to d7ed03c Compare May 31, 2024 21:14
@zkochan
Copy link
Copy Markdown
Member

zkochan commented Jun 3, 2024

Or alternatively the WorkspaceManifest wouldn't be passed at all. Users of @pnpm/workspace.find-packages would pass workspaceManifest.packages as the patterns argument.

This sounds good to me. I guess you could put patterns to the options.

@gluxon
Copy link
Copy Markdown
Member Author

gluxon commented Jun 16, 2024

Or alternatively the WorkspaceManifest wouldn't be passed at all. Users of @pnpm/workspace.find-packages would pass workspaceManifest.packages as the patterns argument.

This sounds good to me. I guess you could put patterns to the options.

Going with this approach in #8213. Since the patterns option already exists, we don't need this PR adding a new workspaceManifest option.

@gluxon gluxon closed this Jun 16, 2024
@gluxon gluxon deleted the cached-workspaceManifest-option branch June 16, 2024 06:44
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