fix: lerna v8 breaking our whole shebangle #2446
Conversation
There was a problem hiding this comment.
Generally looks good, but there are a few places where it appears that lerna is goin g to be invoked in the package root directory, which I think is not correct.
I wonder if it might be a good idea to migrate all the npm scripts that actually invoke lerna from package.json to plugins/package.json and/or examples/package.json, and then have the top level scripts just cd plugins && npm run <script>? That way you can run the same scripts from either directory (at least until we decide we want the top-level versions to run the same thing in both plugins and examples).
(One possible counter to that is that at the moment there seem to be some lerna invocations in the gulpfile(s) which reside at the top level, so removing all instances of "lerna" from the top-level package.json wouldn't result in it only ever being invoked from the appropriate sub-directory's package.json. Perhaps the gulpfile invocations could also be replaced with cd … && npm run … calls?)
| "boot": "cd plugins && npm ci", | ||
| "build": "cd plugins && lerna run build", | ||
| "clean": "cd plugins && lerna run clean", | ||
| "clean:node": "cd plugins && lerna clean --yes", | ||
| "deploy:prepare": "npm run deploy:prepare:plugins && npm run deploy:prepare:examples && gulp predeploy", |
There was a problem hiding this comment.
The use of a direct gulp invocation here prompted me to go and take a peek at the gulpfile.js and its various requires, and though it mostly looks fine I did notice that there are a few places where it directly invokes lerna that will surely need updating. (I don't think the predeploy task hits those, but presumably they are either reachable from other task entrypoints—or they are vestigial, no longer reachable from any task and should be deleted.)
There was a problem hiding this comment.
Ah ok I didn't expect the scripts to be invoking lerna. I'll give that a look.
package.json
Outdated
| "boot": "cd plugins && npm ci", | ||
| "build": "cd plugins && lerna run build", | ||
| "clean": "cd plugins && lerna run clean", | ||
| "clean:node": "cd plugins && lerna clean --yes", | ||
| "deploy:prepare": "npm run deploy:prepare:plugins && npm run deploy:prepare:examples && gulp predeploy", | ||
| "deploy:prepare:examples": "cd examples && npm run predeploy", | ||
| "deploy:prepare:plugins": "npm run clean && lerna run build && lerna run predeploy", |
There was a problem hiding this comment.
Looks like this should have a cd plugins && spliced in before the lerna invocations, unless I've misunderstood something.
There was a problem hiding this comment.
Thanks for the catch!
|
Hopefully nothing breaks tomorrow! |
So the problem was, we've only been doing everything kind of wrong. Our repo was a lerna repo at the top level, and we also had a lerna repo in the
examplesdirectory. Having nested lerna repositories was never supported, and is now actually broken.So how we're going to fix this is just have two side by side lerna repos, one in the
examplesrepository, and one in thepluginsrepository. The reason we're keeping them separate is because our top-level npm scripts currenlty only act on the plugin packages, and we want to maintain that behavior. We do that by delegating the top-level scripts to scripts in the plugins directory.I also tried having lerna only installed at the top level, but having two separate configs, but that is... not supported. It yells at you if you don't have a
lerna.jsonconfig in the directory where you have lerna installed.@cpcallen I'm not sure I migrated all of the top-level scripts correctly. I tested all of the methods I changed and they seemed to work. But would love your scrutiny.