webpack-chain icon indicating copy to clipboard operation
webpack-chain copied to clipboard

Discuss whether we should remove .merge()

Open edmorley opened this issue 5 years ago • 1 comments

The .merge() feature has a number of limitations:

  1. The object passed to it must be a in a very specific schema, that is similar enough to the webpack config schema as to cause confusion and make it seem like a webpack config could be used, even though it won't always work (see #225, #237)
  2. it doesn't support merging all valid forms of webpack config (eg #210, #230)
  3. Whenever we add new webpack-chain features, it increases the complexity of supporting/testing them

Looking at issues that people have reported, it seems that in many cases users are really wanting a clone feature, and are using toConfig() followed by .merge() to attempt to achieve that (even though it doesn't work due to (1) above). For example: https://github.com/neutrinojs/webpack-chain/issues/204#issuecomment-566432553

In the remaining cases, it seems that users are using it as an alternative to the webpack-chain API, and passing in an object instead of making the method calls (eg: https://github.com/neutrinojs/webpack-chain/pull/228#issuecomment-572004951). For me, I'm not a massive fan of supporting multiple ways of achieving the same functionality, particularly if it adds overhead/bugs.

It seems like if we added a real clone feature (filed as #212) then much of the need for .merge() would disappear?

Thoughts? Are there additional use-cases I've missed?

edmorley avatar Jan 29 '20 08:01 edmorley

Are you talking about .merge() on the root config or all .merge()s in things like ChainedMap, ChainedSet, etc?

I think at the root, perhaps .clone() would solve most use cases I can think of (except for migrating from legacy code), but I can see .merge() being useful on ChainedMap and ChainedSet when you want to say, reuse parts of another Rule, like in https://github.com/neutrinojs/webpack-chain/issues/129. You could still .clone() the rule, but getting it back into the config you want it to is not possible without .merge()

Cobertos avatar Apr 28 '20 20:04 Cobertos