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

chore(pnpm-workspace.yaml): packages/* in the pnpm-workspace.yaml is vague, it would be better to list specific package names #9968

Conversation

peterlevel1
Copy link

@peterlevel1 peterlevel1 commented Jan 1, 2024

Purpose: Better for managing packages of vuejs/core

As vuejs/core project grows, it would be better to list specific package names, which can be more clear to see how many packages in the project when using pnpm as package manager.

Why this commit ?

An error shown when running pnpm build:
../packages/reactivity-transform/package.json is not found

Above error gave me a confusing for a short time, and after comparing the remote vuejs/core code repo with local code base, making sure this empty folder should not be appear in the project.

Benefits

If the specific package name listed in the pnpm-workspace.yaml, there would be 2 benefits apparently

  1. More clear when adding package

Adding package would be more clear in the project, as it must be handled by pnpm in the pnpm-workspace.yaml

  1. Packages reference

As packages reference for folders in the packages, which holding so many packages in the vuejs/core project that pnpm-workspace.yaml file can be compared with existed packages in the packages folder.

…vague, it would be better to list specific package names
@yyx990803
Copy link
Member

I don't think I agree. All directories under packages should be treated as a workspace package by pnpm to ensure correct dependencies. Manually listing them only creates room for error in the future (e.g. creating a new package without adding them to the workspace), and this would already require changes to core-vapor which adds a bunch of new packages.

reactivity-transform was removed in the 3.4 release and it's a leftover due to the lingering files not checked into git and thus preserved by git when checking out the newer commit.

@yyx990803 yyx990803 closed this Jan 3, 2024
@peterlevel1
Copy link
Author

Ok, agree

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