Skip to content
This repository has been archived by the owner on Feb 18, 2024. It is now read-only.

Discuss whether we should remove .merge() #240

Closed
edmorley opened this issue Jan 29, 2020 · 1 comment
Closed

Discuss whether we should remove .merge() #240

edmorley opened this issue Jan 29, 2020 · 1 comment

Comments

@edmorley
Copy link
Member

edmorley commented Jan 29, 2020

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 docs: Emphasise that merge() doesn't accept webpack config objects #225, Simplify .merge() examples to make deviations from webpack schema clearer #237)
  2. it doesn't support merging all valid forms of webpack config (eg enhance function rule.merge #210, Support a single webpack entry #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:
#204 (comment)

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: #228 (comment)). 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?

@Cobertos
Copy link

Cobertos commented Apr 28, 2020

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 #129. You could still .clone() the rule, but getting it back into the config you want it to is not possible without .merge()

@edmorley edmorley closed this as completed Feb 3, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Development

No branches or pull requests

2 participants