Fix getting workspace ignores in case of non-monorepo

[stefanofa]

Description

Hey @mehulkar I tried to fix #4140 .
This is my first time reading or writing in go so it's likely I may have done something wrong.

The way I did it is a bit redundant, as in the case of a monorepo the call to getWorkspaceGlobs is made 2 times and so is not the most efficient. Unfortunately I think I lack a lot of context to try and come up with a better solution, but I felt like trying :)

[vercel]

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

Name	Status	Preview	Comments	Updated
examples-designsystem-docs	🔄 Building (Inspect)			Mar 20, 2023 at 9:16PM (UTC)

9 Ignored Deployments

Name	Status	Preview	Comments	Updated
examples-basic-web	⬜️ Ignored (Inspect)			Mar 20, 2023 at 9:16PM (UTC)
examples-cra-web	⬜️ Ignored (Inspect)			Mar 20, 2023 at 9:16PM (UTC)
examples-gatsby-web	⬜️ Ignored (Inspect)			Mar 20, 2023 at 9:16PM (UTC)
examples-kitchensink-blog	⬜️ Ignored (Inspect)			Mar 20, 2023 at 9:16PM (UTC)
examples-native-web	⬜️ Ignored (Inspect)			Mar 20, 2023 at 9:16PM (UTC)
examples-nonmonorepo	⬜️ Ignored (Inspect)			Mar 20, 2023 at 9:16PM (UTC)
examples-svelte-web	⬜️ Ignored (Inspect)			Mar 20, 2023 at 9:16PM (UTC)
examples-tailwind-web	⬜️ Ignored (Inspect)			Mar 20, 2023 at 9:16PM (UTC)
examples-vite-web	⬜️ Ignored (Inspect)			Mar 20, 2023 at 9:16PM (UTC)

[vercel]

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

A member of the Team first needs to authorize it.

[mehulkar]

Hey! thanks for taking a stab at this!

The way I did it is a bit redundant, as in the case of a monorepo the call to getWorkspaceGlobs is made 2 times and so is not the most efficient.

Just making sure, do you mean there's a duplicate call to fs.ReadPackageJSON?

Instead of fs.ReadPackageJSON twice, how about returning a custom error from getWorkspaceGlobs (instead of fmt.Errorf()), and returning the ignores: ["node_modules/**] in that case, but bubbling up all other errors?

Happy to give you another pointer to custom error handling if you want!

PS, I know @tknickman also said he might have time to take a look at this, in case he had a different solution in mind.

[stefanofa]

Just making sure, do you mean there's a duplicate call to fs.ReadPackageJSON?

Yes, that's what I meant.

By the way, I tried the refactor you suggested!

[mehulkar]

Couple more questions:

• What happens in a real monorepo when the workspace config is missing?
• In npm and pnpm implementations of getWorkspaceIgnores, the default return value is **/node_modules/**. Should we use that as the default here also? Or can we trust the user that if the workspaces config is missing, then we really only want to ignore the root ./node_modules?

[chris-olszewski]

What happens in a real monorepo when the workspace config is missing?

This should still error as we call packageManager.GetWorkspaces in the BuildPackageGraph context constructor.

* In `npm` and `pnpm` implementations of `getWorkspaceIgnores`, the default return value is `**/node_modules/**`. Should we use that as the default here also? Or can we trust the user that if the workspaces config is missing, then we really only want to ignore the root `./node_modules`?

I think from my groking that node_modules/** matches what yarn does by default: https://github.com/yarnpkg/yarn/blob/3119382885ea373d3c13d6a846de743eca8c914b/src/config.js#L816

[mehulkar]

Let's merge this into my branch then!