-
-
Notifications
You must be signed in to change notification settings - Fork 5.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Update lerna #10733
Update lerna #10733
Conversation
nicolo-ribaudo
commented
Nov 18, 2019
Q | A |
---|---|
Tests Added + Pass? | Yes |
Documentation PR Link | |
Any Dependency Changes? | Yes |
License | MIT |
package.json
Outdated
@@ -73,7 +73,8 @@ | |||
"webpack-stream": "^4.0.0" | |||
}, | |||
"resolutions": { | |||
"@lerna/**/@lerna/collect-updates": "https://github.com/babel/lerna.git#babel-collect-updates" | |||
"@lerna/**/@lerna/collect-updates": "https://github.com/babel/lerna.git#babel-collect-updates", | |||
"@lerna/cli/yargs": "14.2.0" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Version 14.2.1
generates an error:
TypeError: Cannot delete property '--' of #<Object>
at Object.Yargs.self._copyDoubleDash (/home/nicolo/Documenti/dev/babel/babel/node_modules/@lerna/cli/node_modules/yargs/yargs.js:1196:5)
at Object.parseArgs [as _parseArgs] (/home/nicolo/Documenti/dev/babel/babel/node_modules/@lerna/cli/node_modules/yargs/yargs.js:1097:60)
at Object.parse (/home/nicolo/Documenti/dev/babel/babel/node_modules/@lerna/cli/node_modules/yargs/yargs.js:578:25)
at main (/home/nicolo/Documenti/dev/babel/babel/node_modules/lerna/index.js:44:6)
at Object.<anonymous> (/home/nicolo/Documenti/dev/babel/babel/node_modules/lerna/cli.js:11:15)
at Module._compile (internal/modules/cjs/loader.js:689:30)
at Object.Module._extensions..js (internal/modules/cjs/loader.js:700:10)
at Module.load (internal/modules/cjs/loader.js:599:32)
at tryModuleLoad (internal/modules/cjs/loader.js:538:12)
at Function.Module._load (internal/modules/cjs/loader.js:530:3)
because of
Line 240 in 6ba1131
yarn lerna bootstrap -- -- --ignore-engines |
cc @bcoe (author of the commit in v14.2.1 - yargs/yargs@e78e76e) is this a known issue fixed in v15?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could we work around it by
node $(npm bin)/lerna bootstrap -- --ignore-engines
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@nicolo-ribaudo something outside of yargs, in lerna
is calling Object.seal()
on the argv
object, before the parse is finished (there's a two pass parse that temporarily stores positional arguments in --
and then merges them into the final parse).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@JLHwung Nope:
node node_modules/.bin/lerna bootstrap -- --ignore-engines
/home/nicolo/Documenti/dev/babel/babel/node_modules/@lerna/cli/node_modules/yargs/yargs.js:1184
else throw err
^
TypeError: Cannot delete property '--' of #<Object>
at Object.Yargs.self._copyDoubleDash (/home/nicolo/Documenti/dev/babel/babel/node_modules/@lerna/cli/node_modules/yargs/yargs.js:1196:5)
at Object.parseArgs [as _parseArgs] (/home/nicolo/Documenti/dev/babel/babel/node_modules/@lerna/cli/node_modules/yargs/yargs.js:1097:60)
at Object.parse (/home/nicolo/Documenti/dev/babel/babel/node_modules/@lerna/cli/node_modules/yargs/yargs.js:578:25)
at main (/home/nicolo/Documenti/dev/babel/babel/node_modules/lerna/index.js:44:6)
at Object.<anonymous> (/home/nicolo/Documenti/dev/babel/babel/node_modules/lerna/cli.js:11:15)
at Module._compile (internal/modules/cjs/loader.js:689:30)
at Object.Module._extensions..js (internal/modules/cjs/loader.js:700:10)
at Module.load (internal/modules/cjs/loader.js:599:32)
at tryModuleLoad (internal/modules/cjs/loader.js:538:12)
at Function.Module._load (internal/modules/cjs/loader.js:530:3)
make: *** [Makefile:240: lerna-bootstrap] Error 1
@bcoe Thanks! I'll fix it in lerna then.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@nicolo-ribaudo could you try commenting out this line?
I haven't quite figured out how to make a reproduction in yargs; If we can figure out a reproduction, I'd be happy to add a try
/catch
, and just skip the delete
step for this edge case.
This does not fail, like I would hope:
// see: https://github.com/babel/babel/pull/10733
it('does not fail if Object.seal() has been called on argv', () => {
Object.seal(process.argv)
const argv = yargs()
.command('cmd <version>', 'a command', () => {}, (argv) => {
Object.freeze(argv)
}).argv
console.info(argv);
})
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ack, sorry folks. I don't even remember why I cared about freezing the argv. I can remove it, as it serves basically no purpose.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@evocateur I think you should be able to freeze the object once it's been passed to a command ... so it feels like yargs stepping on your toes as well.
I was thinking this morning, perhaps we pass a shallow clone of the object to the command rather than the actual argv
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm fixing it in lerna/lerna@8ca85a4 by deep cloning argv
before I mutate it. Releasing momentarily...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lerna v3.18.5 published with the aforementioned fix
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks @evocateur!
e4698c6
to
33014fc
Compare
See babel/babel#10733 for details. Fixes #2348
The problem encountered in babel/babel#10733 has been fixed, but bump it anyway.
3f90a33
to
4d490b6
Compare
4d490b6
to
082c06e
Compare