feat(core): workspace resolution respects gitignored files/dirs

[joemckenney]

What's the problem this PR addresses?

See #4572

How did you fix it?

As mentioned in the issue linked above, one solution (albeit non-backward compatible) would be to use pre-existing configuration exposed by the globbing library y'all utilize for workspace resolution. In particular globby has config for respecting gitignore.

Checklist

• I have read the Contributing Guide.

• I have set the packages that need to be released for my changes to be effective.

• I will check that all automated PR checks pass before the PR gets reviewed.

[merceyz]

I don't think we should change the workspaces field to also be controlled by external files, instead we should investigate why the negated pattern didn't work for you.

Note that you've marked this change as a patch, the PR title / commit says it's minor, and the description says it's major, it can't be all three at the same time.

[joemckenney]

I don't think we should change the workspaces field to also be controlled by external files, instead, we should investigate why the negated pattern didn't work for you.

It's a fair point. My thoughts here:

• I wonder if anyone would expect the globs in workspaces to match gitignored files. Like I mentioned in the issue, it was very unexpected.
• The negation glob in our workspace config feel like implementation details of yarn workspaces spilling out into our app. I'm not sure any dev on the team not aware of this bug would understand why it was there if they encountered it e.g. "Should all build artifact directories that are gitignored be listed here? If not, why?"

That all being said, the negation is a more elegant fix than what we are currently doing i.e. we rm -rf the generated package.json that is tucked away in the gitignored dir immediately after it is generated. This isn't bulletproof as our build is multi-threaded so any parallel yarn call that hits the refreshWorkspaceDependencies() code path while the package.json is there will fail with Assertion failed: Expected workspace...

I can take another pass at trying to understand globby's negation through their code as their docs don't offer much.

Note that you've marked this change as a patch, the PR title / commit says it's minor, and the description says it's major, it can't be all three at the same time.

Very fair. Strictly speaking, this is major.

[joemckenney]

I can take another pass at trying to understand globby's negation through their code as their docs don't offer much.

Based on globby's implementation of ignores, y'alls configuration of globby, and prisma's generation of package.json in their build artifacts a "fix" via workspace globs for our usage looks like this

  "workspaces": {
    "packages": [
      "apps/**/*",
      "packages/**/*",
      "services/**/*",
      "!**/dist/**"
    ]
  },

Not great. It reinforces my belief that this should probably be fixed in yarn.

[merceyz]

Closing this due to #4574 (review)

---

Instead of using negated patterns you could use a less inclusive pattern.

{
	"workspaces": [
		"apps/*",
		"packages/*",
		"services/*"
	]
}