Thinkmill / manypkg

☔️ An umbrella for your monorepo
MIT License
884 stars 48 forks source link

Prefer workspace protocol when using pnpm #72

Closed ifiokjr closed 5 months ago

ifiokjr commented 4 years ago

Description

When pnpm encounters a package in a workspace which has a lower version than the most recently published on the npm registry it defaults to using the registry version.

I opened an issue and the proposed solution was to use workspace protocols.

Unfortunately, manypkg fix removes all workspace protocols when run.

My proposal is not just to ignore workspace protocols when using pnpm but actually to prefer them, since it's the recommended way to prevent a hard situation to debug. It took me a while to realize why none of my changes were having an impact.

maraisr commented 2 years ago

@mitchellhamilton @Andarist would be so awesome to get an answer here. More than happy to explore at a solution as well.

Andarist commented 2 years ago

I assume that the proposal here is for fix to enforce the workspace protocol (when used with pnpm). What about check? Should it allow both? Warn when it encounters non-workspace dependency? I've seen people mixing workspace package with non-workspace ones intentionally (the cost of simultaneous upgrades in large monorepos was too high for them). OTOH the "Internal mismatch" rule explicitly forbids this - however, with workspace protocol one can have a more explicit and greater control over this. So maybe in this context it's not worth enforcing this rule so strictly - without the workspace protocol (yarn v1, npm?) this was much a bigger issue because one couldn't control this in such a way.

maraisr commented 2 years ago

@Andarist what is the go here? Can I raise a pr to fix something? Just need to know what needs to happen.

chaance commented 2 years ago

I'm also running into this. Has anyone found a workaround since the issue was originally raised?

https://github.com/Thinkmill/manypkg/blob/b8b914441995ad267a490251d443665fb999e32b/packages/cli/src/checks/INTERNAL_MISMATCH.ts#L23

Starting in the INTERNAL_MISMATCH check, simply checking for and stripping the protocol works great for me locally.

let range = deps[depName]; 
if (range.startsWith('workspace:') {
  range = range.slice(10);
}

I'm unsure if this is a good solution but am more than happy to make a PR if I'm on the right track here @Andarist. I know Yarn PnP also uses the workspace: protocol, but I'm unsure if there are others we'd need to check well.

For now, I'm just ignoring the INTERNAL_MISMATCH rule altogether in package.json.

{
  "manypkg": {
    "ignoredRules": [
      "INTERNAL_MISMATCH"
    ]
  }
}
mattpocock commented 2 years ago

Wanted to flag this since it makes manypkg kind of unusable with pnpm. Would you accept a PR on this?

Andarist commented 2 years ago

Yeah, I think a PR for this would be accepted. At the very least we should allow those ranges and don't raise the INTERNAL_MISMATCH error. We should have tests for workspace:*, workspace:^, workspace:~ and workspace:1.2.3. Because workspace:^ and workspace:~ ranges are valid a simple .slice like mentioned by @chaance might not be enough - those could be mapped to just * though.

OTOH * doesn't allow prerelease versions (IIRC there is a flag available to semver methods that can accept them but it still might not be enough to handle things correctly, it would have to be tested though).

As requested by @ifiokjr - we should also have a config flag (TIL that package.json#manypkg is a thing) to prefer workspace ranges. With such a setting we should raise appropriate errors and offer an auto-fixer

rtman commented 1 year ago

any progress on this issue? Having this problem with yarn workspaces.

antoniopresto commented 1 year ago

I have a small utility called runmate and i just added support for deps sync

webpro commented 1 year ago

Opened #186 to tackle this.