Make yarn --no-lockfile optional

[oscarni]

We've encountered an issue where we have a dependency which makes our tests fail. It's very annoying and it would be great if --no-lockfile could be optional, preferably set by default and you have the option to respect the lock file in a scenario if you'd like.

We use ember-try to test different builds of our ember-app. We use ember-cli-conditional-compile in order to create different builds for different websites with different features and to test the different builds we use ember-try's different scenarios. It works great for us in most cases, but it feels like ember-try is only focusing on different scenarios where you test ember betas or other versions of ember data, etc. In those cases --no-lockfile makes a lot of sense. But for us in order to test our different conditionally compiled scenarios we would be prefer if we could test the different compilations with the lockfile so we can make sure the tests are run with the exact same deps and the same deps as the production build will have.

A setting like --respect-lockfile or some similiar naming could work great. I'd be happy to give it a whirl and create a PR if you find this approach reasonable?

[rwjblue]

Seems reasonable to skip the npm/yarn install step when no package changes are made.

@kategengler - what do you think?

[rwjblue]

I think that the nomOptions option should default to —no-lockfile —ignore-engines (when using yarn) and —no-shrinkwrap (when using npm), but if the user provides a custom value (at all) we use it without modification.

Currently, we always add to the options and there is (as pointed out above) no way to opt out

ember-try/lib/dependency-manager-adapters/npm.js

Lines 102 to 120 in 4f8b32a

	if (this.useYarnCommand) {
	if (mgrOptions.indexOf('--no-lockfile') === -1) {
	mgrOptions = mgrOptions.concat(['--no-lockfile']);
	}
	// npm warns on incompatible engines
	// yarn errors, not a good experience
	if (mgrOptions.indexOf('--ignore-engines') === -1) {
	mgrOptions = mgrOptions.concat(['--ignore-engines']);
	}
	} else if (mgrOptions.indexOf('--no-shrinkwrap') === -1) {
	mgrOptions = mgrOptions.concat(['--no-shrinkwrap']);
	}
	return this.run(cmd, [].concat(['install'], mgrOptions), { cwd: this.cwd }).then(() => {
	if (!adapter.useYarnCommand) {
	return adapter.run(adapter.configKey, ['prune'], { cwd: adapter.cwd });
	}
	});
	},

[kategengler]

I think skipping the install when no dep changes are made makes sense. That ties in with #143 a bit and requires a bit of a big refactor (which I think is due anyway because ember-try apparently isn't just about changing dependencies anymore).

I'm less sure about having npmOptions override, since it would be a breaking change. What about a new option managerOptions that would be used if set in place? It's a better name now that it is used for npm & yarn and won't break anybody's existing config.

I do think that we'll need to stress that when testing different dependencies (especially ember versions) that ignoring the lockfile is the best way to truly test that those dependency change work.

[rwjblue]

I'm less sure about having npmOptions override, since it would be a breaking change. What about a new option managerOptions that would be used if set in place? It's a better name now that it is used for npm & yarn and won't break anybody's existing config.

Sounds good to me...

[rwjblue]

This was fixed by #409 (via buildManagerOptions) and is documented in the README.