--entry should override config.entry

[bitpshr]

What kind of change does this PR introduce?
Feature

Did you add tests for your changes?
Yes

If relevant, did you update the documentation?
Yes

Summary
It should be possible to use a configuration file while also overridding specific parameters using CLI options. This is already possible for most configuration options. For example, the CLI will ignore any output configured in a project's webpack.config.js and instead use the value of --output if it's passed. This allows a configuration file to be used while also allowing certain options to be overridden dynamically at runtime. Unfortunately, the current implementation doesn't handle entry in this manner. Instead, the CLI will append the value of any --entry option to the entry configuration in a project's webpack.config.js instead of overriding, which prevents projects from dynamically overriding entry points using CLI options.

This PR updates the CLI to handle --entry the same way as --output: any entry points specified via CLI options override entry configuration specified in a configuration file, rather than appending to it.

Resolves #155

Example
webpack.config.js

module.exports = {
	entry: {
		foo: './some/path.js'
	}
};

Should create one foo entry based on config file:

webpack-cli

Should override config file and create one bar entry:

webpack-cli --entry bar=./some/path.js

or

webpack-cli bar=./some/path.js

Does this PR introduce a breaking change?
Eh, sort of. If projects were previously relying on the undocumented functionality where CLI-passed entry points were appended to configuration-defined entry points, they will break with this release. I'd argue that since the previous functionality wasn't explicit or documented that it doesn't warrant a major version bump.

Other information
This PR also updates the usage help text printed by the CLI since it wasn't exactly accurate. Using CLI options is not mutually-exclusive to using a configuration file, and an entry point can be dynamically defined as either a CLI option or a CLI argument.

[webpack-bot]

Thank you for your pull request! The most important CI builds succeeded, we’ll review the pull request soon.

[evenstensberg]

Looks good, thanks! I'm a bit worried about the removal of the Array checking, but tests are running fine. Could revert if its a problem.

[fokusferit]

Could we also provide a test regarding multiple entry overrides? So we are also sure that providing entry Arrays is also overridden as expected.

[evenstensberg]

@fokusferit this might be implemented already, could you check the binTestCases?

[fokusferit]

Also I'm currently thinking about this line:

			function(name, entry) {
				if (
					typeof options.entry[name] !== "undefined" &&
					options.entry[name] !== null
				) {
					options.entry[name] = [].concat(options.entry[name]).concat(entry);
				} else {
					options.entry[name] = entry;
				}
			}

Is this if-else still necessary or valid?

We are forcing that entry is options.entry = {}, we also know then that options.entry[name] will always be the else block, right?

It's in another subfolder https://github.com/webpack/webpack-cli/tree/master/test/binCases/entry/multi-file

[bitpshr]

@ev1stensberg I removed the array check because if we get to that point in the code, we know that entries were passed as CLI arguments, so we force options.entry to be an empty object anyways. The array check should no longer be needed. Good point to mention though: this implementation means that CLI argument entries take precedence over CLI option entries, which in turn take precedence over config file entries. This was intentional, as a user shouldn't ever be passing both CLI arguments and CLI options for entry, e.g. webpack-cli bar=./some/path.js --entry bar=./another/path.js. They should just choose one syntax.

@fokusferit I left the conditional check you referenced in the code because that function as a whole appears to be called for every entry provided via CLI options. I figured we should still allow a downstream user to provide multiple paths for the same entry:

webpack-cli --entry bar=./some/path.js --entry bar=./another/path.js

Thanks for the review! Happy to rework any of this, just let me know.

[fokusferit]

@bitpshr thanks for the explanation! Yes, I've just quickly asked myself which reason is left to keep that code and thanks for the use case.

I don't have any comment so, when @ev1stensberg is still fine, we can merge it.

[bitpshr]

Thanks @ematipico, @fokusferit, and @ev1stensberg. Let me know if you'd like me to clarify or clean up anything else before landing this.

[shellscape]

I'm kinda shocked that no one raised an issue with this PR. This effectively means that users can’t add entries dynamically on the command line, which is the pattern every flag but --output follows. Architecturally speaking, it would have been more beneficial to users to provide a flag instructing the cli to override, rather than changing the behavior.

This PR only exchanges one set of disadvantages for another. Poor choice imho, folks.

[bitpshr]

@shellscape I'm not sure I follow; most CLI options passed dynamically at runtime override corresponding configuration properties defined in a configuration file. See --output, --context, --watch, --mode, --target, --devtool, etc., not to mention all output-related configuration properties beyond just --output.

While I understand the desire to dynamically add to configuration-file-defined entry points via the CLI instead of overriding them as was previously possible (albeit undocumented), this makes it so it's simply not possible to override entry points at all without the use of a second configuration file. This PR results in one API change for dynamic entries: if a specific use case requires that entries are dynamically added at runtime via the CLI, all entries will have to be passed via the CLI, not just those that are dynamic.

I disagree with the assertion that this PR trades one set of disadvantages for another: this PR makes dynamically adding to entries slightly more tedious so that overriding entries is at all possible, and does so by handling entry like most other CLI configuration options.

[shellscape]

I'm not sure you have a full handle on the scope of the CLI options available, because "most" is not a true assertion. By your logic you should then make all flags that modify the plugins collection override and wipe away any and all plugins specified in the config.

this PR makes dynamically adding to entries slightly more tedious so that overriding entries is at all possible

That illustrates my point perfectly - that is literally trading one inconvenience for another. This PR was not forward thinking, and merged with tunnel vision for only one way of doing things, whilst ignoring the other. It's a failure of vision and an architectural mistake.

[bitpshr]

Ah, so we handle any modifications to the plugins configuration in an additive manner as well. I see your gripe with the asymmetry there. Granted, that's a slightly contrived piece of configuration to use to highlight your issue with the approach; for example, passing --hot has to modify the plugins configuration by necessity, but most users would not expect this to override all other plugins. I'd also argue that most users passing a --plugin option are doing so specifically to dynamically load an additional plugin, not to override and define a new plugin configuration altogether (as is seemingly more common / expected with entry, hence #155 and its upvotes.)

Is plugins the only remaining piece of configuration that's added to via associated CLI options instead of being overridden by them? If so, and you see the asymmetric handling of plugins compared to entry and most other configuration options as problematic, and for the sake of being constructive, feel free to open an issue to discuss the larger question of how such CLI options should be handled:

1. Should CLI options always override configuration file options, except for plugins? (current behavior)
2. Should CLI options always override configuration file options, including plugins?
3. Should CLI options always extend configuration file options where possible?
4. Should CLI options conditionally override or extend configuration file options based on a flag?

Thanks for bringing up up your concern. I worry we can only be so consistent when translating CLI options to configuration options, with plugins potentially being a special case, but an issue to track alternative approaches may be worthwhile.