chore: nx migrate plus keep lock files #10536
Conversation
| @@ -7,7 +7,6 @@ | |||
|
|
|||
| # dependencies | |||
| **/node_modules | |||
| **/package-lock.json | |||
There was a problem hiding this comment.
Although I think npm is the slowest package manager at installing, it's indeed the one with the lowest barrier to entry for contributors (it's definitely installed), so probably the right one to standardise on 👍
(if only the package managers could agree on a common lockfile format 🥲)
| "overrides": { | ||
| "@nx/devkit": "18.2.2", | ||
| "@nx/eslint-plugin": "18.2.2", | ||
| "@nx/jest": "18.2.2", | ||
| "@nx/js": "18.2.2", | ||
| "@nx/node": "18.2.2", | ||
| "@nx/plugin": "18.2.2", | ||
| "@nx/workspace": "18.2.2", | ||
| "nx": "18.2.2" | ||
| }, |
There was a problem hiding this comment.
Nice that we're able to drop these!
packages/webpack5/package-lock.json
Outdated
There was a problem hiding this comment.
In nx monorepos, does each package get its own package lock? Just that in npm workspaces and pnpm monorepos, they don't, so checking that it's right for this to be here at all.
There was a problem hiding this comment.
webpack5 is only exception here since we still manage it's package in isolated fashion. We can move away from that soon but it won't be addressed in this PR.
apps/automated/package-lock.json
Outdated
There was a problem hiding this comment.
Looks correct. It's using the specified local packages (e.g. @nativescript/core), it's the latest lockfileVersion, and pulls from registry.npmjs.org.
There was a problem hiding this comment.
We don't need them in apps, I"ll add ignore for this - doesn't hurt either but agreed not needed in apps - these are essentially demo apps to support testing/experimentation/verifying of the packages.
There was a problem hiding this comment.
Looks like a great improvement, thanks for taking the time! @NathanWalker
I'm approving, assuming the answer to this is "yes, each package does get a lockfile":
In nx monorepos, does each package get its own package lock? Just that in npm workspaces and pnpm monorepos, they don't, so checking that it's right for this to be here at all.
If each package is actually not supposed to have its own lockfile, then let's get to the bottom of that before proceeding.
|
Actually, that's a point. Either all packages and apps should have their own lockfile, or none of them. I notice that the lockfiles for It seems I can't withdraw my PR approval for now, so could you please just check on that (or address in a follow-up PR)! |
|
There was a problem hiding this comment.
Having discussed on Discord that the package-lock for Webpack5 is present because it's not a properly integrated member of the monorepo (i.e. not a true workspace), I'm happy to approve. Will be happy for Webpack5 to be properly integrated in a follow-up PR in future.
PR Checklist