Add buildManagerOptions config option (supports opting out of --ignore-lockfile)
#409
Conversation
Codecov Report
@@ Coverage Diff @@
## master #409 +/- ##
==========================================
+ Coverage 96.72% 96.87% +0.14%
==========================================
Files 17 17
Lines 550 576 +26
==========================================
+ Hits 532 558 +26
Misses 18 18
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Instead of adding another flag, we really want a way to specify exactly what arguments to use with the package manager. I think that would allow more overall capabilities and be easier in the implementation.
Concretely, I think this would be a decent API:
// config/ember-try.js
module.exports = async function() {
return {
buildNpmOptions(scenarioName) {
return []; // opt-out of defaulting `--ignore-engines --no-lockfile`
},
scenarios: [
}
};
};Then we could structure dependency-manager-adapters/npm.js, such that in _install it would call the users function to get the final list of options.
|
I chatted with @kategengler, she suggested we use |
|
I see...so if someone has buildManagerOptions in their config, then we use that config instead of the default. If shrinkwrap or yarn.lock files are detected, then we back up those files and restore accordingly. I can make that change. |
|
Ya, exactly |
| if (mgrOptions.indexOf('--ignore-engines') === -1) { | ||
| mgrOptions = mgrOptions.concat(['--ignore-engines']); | ||
| } else { | ||
| if (this.useYarnCommand) { |
There was a problem hiding this comment.
Do we intend to mandate the buildManagerOptions in the future? If so, we can add a deprecation here that encourages to use the manager options for these defaults.
There was a problem hiding this comment.
Do we intend to mandate the buildManagerOptions in the future?
No, I believe that it will always be a "power user" option. I would expect that most users would use whatever our default command line options are, and you'd only specify buildManagerOptions if you had to deviate from that for some reason.
There was a problem hiding this comment.
I agree; it is backward compatible and users do not need to use it if there isn't a need.
| if (mgrOptions.indexOf('--ignore-engines') === -1) { | ||
| mgrOptions = mgrOptions.concat(['--ignore-engines']); | ||
| } else { | ||
| if (this.useYarnCommand) { |
There was a problem hiding this comment.
Do we intend to mandate the buildManagerOptions in the future?
No, I believe that it will always be a "power user" option. I would expect that most users would use whatever our default command line options are, and you'd only specify buildManagerOptions if you had to deviate from that for some reason.
There was a problem hiding this comment.
The code changes here look good to me, I think the only things left are documentation changes (update the README.md to include this option and explain what it does) and a review pass by @kategengler.
|
@kategengler @rwjblue Addressed all comments and added buildManagerOptions to the readme. |
rwjblue
changed the title
buildManagerOptions config option (supports opting out of --ignore-lockfile)
|
Released in v1.3.0. |
Currently, ember-try is running yarn with the --no-lockfile option, which will install packages whose version may differ from the actual current yarn.lock file in the project. To get a true dependency environment during testing, this PR aims to add support for using the project's yarn.lock file when using ember-try by adding a config flag called useYarnLockFile that users can pass in to determine whether to use the project's yarn.lock file.