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

Add support for "workspaces" config objects in yarn #5439

Closed
wants to merge 3 commits into from

Conversation

eykrehbein
Copy link

@eykrehbein eykrehbein commented Jul 1, 2023

Description

To be able to use the nohoist config in yarn workspaces, an object with the keys nohoist and packages needs to be provided for the "workspaces" entry in package.json.

The proposed changes add support for a non-array workspaces entry in package.json when using yarn. Before that, only arrays were supported which let to the following exception inside the expandWorkspaces function when using an object:

$ yarn turbo gen <generator>
>>> Unexpected error. Please report it as a bug:
r.flatMap is not a function

A function (parseWorkspacePackages) was introduced to dynamically parse the "workspaces" config by reusing the logic that was already introduced to parse pnpm configs.

Testing Instructions

The given test for yarn was adapted to use a workspaces object instead of an array

@vercel
Copy link

vercel bot commented Jul 1, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
examples-basic-web ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jul 13, 2023 7:07am
examples-cra-web ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jul 13, 2023 7:07am
examples-designsystem-docs ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jul 13, 2023 7:07am
examples-gatsby-web ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jul 13, 2023 7:07am
examples-kitchensink-blog ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jul 13, 2023 7:07am
examples-native-web ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jul 13, 2023 7:07am
examples-nonmonorepo ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jul 13, 2023 7:07am
examples-svelte-web ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jul 13, 2023 7:07am
examples-tailwind-web ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jul 13, 2023 7:07am
examples-vite-web ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jul 13, 2023 7:07am
turbo-site ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jul 13, 2023 7:07am

@vercel
Copy link

vercel bot commented Jul 1, 2023

@eykrehbein is attempting to deploy a commit to the Vercel Team on Vercel.

A member of the Team first needs to authorize it.

@arlyon
Copy link
Contributor

arlyon commented Jul 6, 2023

Hm, odd behaviour from yarn to say the least but I am open to including this. @tknickman is the generator master but he is away right now so I will take over. Going to run CI first then have a look, thanks!

@eykrehbein
Copy link
Author

@arlyon Is there anything I can do from my side right now? 🙂

@arlyon
Copy link
Contributor

arlyon commented Jul 13, 2023

I am merging in main to this branch to see if it helps with CI. This 'symmetric difference' error pops up every once in a while.

Copy link
Member

@chris-olszewski chris-olszewski 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 adding support for this! Only two minor changes and I think this is ready to go. Sorry for the delay on the review.

Comment on lines +111 to +112
if (workspaceConfig instanceof Object) {
return parseWorkspacePackages({ workspaceConfig });
Copy link
Member

Choose a reason for hiding this comment

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

pnpm doesn't need to share this codepath as pnpm-workspace.yaml has it's own schema.

Comment on lines +60 to +62
globs: (packageJson.workspaces as Array<string> | undefined) || [],
workspaces: expandWorkspaces({
workspaceGlobs: packageJson.workspaces,
workspaceGlobs: packageJson.workspaces as Array<string> | undefined,
Copy link
Member

Choose a reason for hiding this comment

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

npm also supports declaring workspaces via "workspaces": { "packages": ["package/a"] } so we should use the parseWorkspacePackages method you added. source

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants