mergeWithRules and different rules for different properties

[just-jeb]

In continuation to #167.
What would be the expected output to the following two objects merge?

const conf1 = {
      module: {
        rules:[
          {
            "test": "/\\.scss$|\\.sass$/",
            "use": [
              {
                "loader": "sass-loader",
                "options": {
                  "sourceMap": true
                }
              }
            ]
          }
        ]
      }
    };
    const conf2 = {
      module: {
        rules:[
          {
            "test": "/\\.scss$|\\.sass$/",
            "exclude": ["/node_modules/"],
            "use": [
              {
                "loader": "sass-resources-loader",
                "options": {
                  "resources": [
                    "src/styles/includes.scss"
                  ]
                }
              }
            ]
          }
        ]
      }
    };

with the following rules?

    const mergeRules = {
      module: {
        rules: {
          test: CustomizeRule.Match,
          use: {
            loader: CustomizeRule.Match,
            options: CustomizeRule.Merge,
          },
        },
      },
    }

And what if I change the rules to the following?

    const mergeRules = {
      module: {
        rules: {
          test: CustomizeRule.Match,
          excluded: CustomizeRule.Append,
          use: {
            loader: CustomizeRule.Match,
            options: CustomizeRule.Merge,
          },
        },
      },
    }

Is this even legit?

[bebraw]

Since both test and loader won't match for the first one, I would expect it to go with the default behavior and merge as separate rules within the same array.

The second one is harder as Append is in-between but I might go with the same here and look for the total match. The in-between case didn't come up before so I'm not sure if that should even be valid.

[just-jeb]

Since both test and loader won't match for the first one, I would expect it to go with the default behavior and merge as separate rules within the same array.

I'd expect the same, but that's not what's happening. exclude does not appear in the resulting object at all...

[just-jeb]

Actually, not quite correct... The test matches but the loader doesn't. But exclude is the test's sibling, not a child, so I'm not sure what's the expected behavior.

I mean, what you said would be a perfect solution for me, since this is what I would expect from the webpack configs merge.
But does it still stand true if we look at it as a generic merge?

[bebraw]

Yeah, I need to add some test for this case to lock the behavior.

On 16.12.2020, at 20:22, JeB notifications@github.com wrote:

 Actually, not quite correct... The test matches but the loader doesn't. But exclude is the test's sibling, not a child, so I'm not sure what's the expected behavior.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub, or unsubscribe.

[just-jeb]

So what would it be? Two separate entries in the rules array or something else?

[bebraw]

Two separate entries might be the least surprising choice.

On 16.12.2020, at 20:43, JeB notifications@github.com wrote:

 So what would it be? Two separate entries in the rules array or something else?

— You are receiving this because you commented. Reply to this email directly, view it on GitHub, or unsubscribe.

[just-jeb]

Looking forward to seeing it! Thanks!

[just-jeb]

Another question. What if we had a match in both test and loader but had additional fields that are different (like exclude and include).
How should it behave then?

const conf1 = {
      module: {
        rules:[
          {
            "test": "/\\.scss$|\\.sass$/",
            "include": ["/node_modules/"]
            "use": [
              {
                "loader": "sass-loader",
                "options": {
                  "sourceMap": true
                }
              }
            ]
          }
        ]
      }
    };
    const conf2 = {
      module: {
        rules:[
          {
            "test": "/\\.scss$|\\.sass$/",
            "exclude": ["/node_modules/"],
            "use": [
              {
                "loader": "sass-loader",
                "options": {
                   anotherOption: "blah"
                }
              }
            ]
          }
        ]
      }
    };

    const mergeRules = {
      module: {
        rules: {
          test: CustomizeRule.Match,
          use: {
            loader: CustomizeRule.Match,
            options: CustomizeRule.Merge,
          },
        },
      },
    }

[bebraw]

@just-jeb That's a good one I need to test. I think it should merge those two matches and fall back to the default merge behavior for anything outside of the explicit definition. That means it would pick up both include and exclude. Of course semantically it's a contradiction and I don't know what webpack will do in this case (which will win?).

It's a good example of the flexibility of webpack loader configuration. In my personal use, I always go with include as it's more explicit and predictable than exclude.

[just-jeb]

Doesn't it contradict a bit the previous example?
On one hand, we say that if there is just a partial match, (like in the first post) then we go with the highest common ancestor (the rules in this case).
On the other hand we say that if there is a full match then we merge the matching object while merging everything along the way with the default behavior. In this case we should at least provide the option to configure the way the properties are merged along the way, don't you think?

I think I'm missing a solid definition of how the mergeWithRules should work and what is the expected behavior. I mean the docs say:

To support advanced merging needs (i.e. merging within loaders), mergeWithRules includes additional syntax that allows you to match fields and apply strategies to match.

But it doesn't really explain how it should work. If it at least explained how it works as for today (based on the implemented algorithm) it would be really helpful I think.

[bebraw]

213 just landed. It's possible it helps with this particular use case or at least can help to find a solution for it.