Allow appending objects in mergeWithRules

[just-jeb]

Consider the following example:

const a = {
  module: {
    rules: [
      {
        test: /\.css$/,
        use: [{ loader: "style-loader", options: { someOption: true } }, { loader: "sass-loader" }],
      },
    ],
  },
};
const b = {
  module: {
    rules: [
      {
        test: /\.css$/,
        use: [
          {
            loader: "style-loader",
            options: {
              modules: true,
            },
          },
        ],
      },
    ],
  },
};
const result = {
  module: {
    rules: [
      {
        test: /\.css$/,
        use: [
          {
            loader: "style-loader",
            options: {
              someOptions: true,
              modules: true,
            },
          },
          { loader: "sass-loader" },
        ],
      },
    ],
  },
};

assert.deepStrictEqual(
  mergeWithRules({
    module: {
      rules: {
        test: "match",
        use: {
          loader: "match",
          options: "append",
        },
      },
    },
  })(a, b),
  result
);

Currently this will fail, since append only supports arrays. However, merging loaders options is a perfectly valid scenario and should be pretty easy to support.
Any chance we can get that?

[bebraw]

I wonder if we should add a new option called "merge" to handle this case. Overloading "append" feels a bit weird. Would that work?

…

On 10.12.2020, at 16:28, JeB ***@***.***> wrote:  Consider the following example: const a = { module: { rules: [ { test: /\.css$/, use: [{ loader: "style-loader", options: { someOption: true } }, { loader: "sass-loader" }], }, ], }, }; const b = { module: { rules: [ { test: /\.css$/, use: [ { loader: "style-loader", options: { modules: true, }, }, ], }, ], }, }; const result = { module: { rules: [ { test: /\.css$/, use: [ { loader: "style-loader", options: { someOptions: true, modules: true, }, }, { loader: "sass-loader" }, ], }, ], }, }; assert.deepStrictEqual( mergeWithRules({ module: { rules: { test: "match", use: { loader: "match", options: "append", }, }, }, })(a, b), result ); Currently this will fail, since append only supports arrays. However, merging loaders options is a perfectly valid scenario and should be pretty easy to support. Any chance we can get that? — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub, or unsubscribe.

[just-jeb]

Sure I thought about it too. It's either this or make append and prepend work excatly the same for objects. But merge sounds good as well.
Also would be nice to get proper exception for certain option not being allowed for objects/arrays. Today if I use append on object I'd get “concat is not a function".

[bebraw]

Also would be nice to get proper exception for certain option not being allowed for objects/arrays. Today if I use append on object I'd get “concat is not a function".

Yeah, I can add a type check and throw for sure.

[bebraw]

@just-jeb I added the merge feature in 5.6.0.

When looking at the code, I realized I cannot add type check against objects/arrays for the case you had in mind as webpack-merge doesn't know anything about webpack's typings. In technical terms, the name webpack-merge is misnomer now that in reality it's a generic merge that happens to be useful with webpack. Occasionally I use it for merging other configurations as well.

It's likely best to use something like TypeScript for checking general webpack configuration as it's definitely beyond the scope of webpack-merge.

[just-jeb]

I understand. Instead of type checking Webpack types though you could check for object/array. Regardless of the application (whether you merge webpack config or something else) you can limit the append/prepend to arrays only and merge to objects only. This way you'll have proper warnings/errors while avoiding things like "concat is not a function". And it also will be pretty easy to document. WDYT?

[bebraw]

I understand. Instead of type checking Webpack types though you could check for object/array. Regardless of the application (whether you merge webpack config or something else) you can limit the append/prepend to arrays only and merge to objects only. This way you'll have proper warnings/errors while avoiding things like "concat is not a function". And it also will be pretty easy to document. WDYT?

Yeah, I see what you mean now. I added simple type checks to guard against append/prepend/merge. I think that covers it well and now it's easier to notice if you are trying to merge types that the system doesn't expect.

If you notice missing checks, open separate issues with small test cases for them and I'll add logic based on that.