mergeWithRules and shortcuts (loaders and use)

[just-jeb]

Two questions:

1. How would one merge loaders with use using mergeWithRules. Is it even possible? Any alternative?
2. The following test fails with TypeError: Cannot read property 'options' of undefined.

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

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

    expect(mergeWithRules(mergeRules)(conf1, conf2)).toBeTruthy();

I'm not sure about that, but I wouldn't expect it to fail. WDYT?

[bebraw]

How would one merge loaders with use using mergeWithRules. Is it even possible? Any alternative?

As I mentioned earlier, webpack-merge doesn't know anything about webpack configuration structure so it cannot do any sort of association between different properties that mean the same thing. It's a bit unfortunate webpack's configuration scheme is so flexible and often there are many ways to achieve exactly the same result.

I had "smart" merging in place in an earlier version that did something along this but in the end it was a nightmare to maintain and in the end the different cases were too much to handle so I went with a simpler design that gives more control (essentially the new merge with rules feature).

The following test fails with TypeError: Cannot read property 'options' of undefined.

What would be the expected result in this case? As I mentioned above, there's no logic within webpack-merge to deal with the discrepancies within webpack configuration.

Now that I think of it, there might be a nice way to handle the problem in the user space by normalizing the configuration. What I mean is that you could have a conversion function loadersToUse that will convert the second configuration to the first format so it's matching your rules.

Regarding the TypeError: Cannot read property 'options' of undefined error, I could throw something nicer in this case.

Does that sound like a plan?

[just-jeb]

@bebraw Thanks for the swift response! I understand the generic nature of webpack-merge, just wondered if there is any way to achieve that with mergeWithRules. I like the idea of the transformation function, should definitely give it a try.
Regarding the exception, could you please elaborate on why it happens? Given the two configurations and the rules in the example?

[bebraw]

I understand the generic nature of webpack-merge, just wondered if there is any way to achieve that with mergeWithRules.

There's no way within the function. Patching webpack configuration feels like a too specific task for it.

Regarding the exception, could you please elaborate on why it happens? Given the two configurations and the rules in the example?

I think it's looking for the options field that doesn't exist because use doesn't exist in the latter configuration.

It's a good case for me to catch.

[just-jeb]

As a matter of fact if I change the loaders to use then I still get this exception. I think there is a problem with a matching logic.

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

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

    expect(mergeWithRules(mergeRules)(conf1, conf2)).toBeTruthy();

If in addition I change the merge rules to the following, the test will pass:

const mergeRules = {
      module: {
        rules: {
          test: CustomizeRule.Match,
          use: CustomizeRule.Append
        },
      },
    }

Seems like a bug to me, am I wrong?

[bebraw]

@just-jeb Are you using 5.7.1? I just fixed this kind of issue this morning.

[just-jeb]

The problem here is that there is a partial match. I'm not even sure what's the expected behavior in this case...
Let's say I want to merge the options if there is a full match (test + loader) and append the use if there is a partial match (just test).
How would I do that?
And yes, I use 5.7.1.

[bebraw]

Likely for a partial match, it shouldn't do anything as that's how it's defined to work. It's a good question if it should throw then or do something else.

[bebraw]

What I mean is that maybe one good option for non-matches would be to use the default merge behavior.

[just-jeb]

Which is append for arrays, correct?
So what is the way to achieve this:

Let's say I want to merge the options if there is a full match (test + loader) and append the use if there is a partial match (just test).

[bebraw]

Which is append for arrays, correct?

Yup, it would append as a new rule.

Let's say I want to merge the options if there is a full match (test + loader) and append the use if there is a partial match (just test).

I am not sure how to support the partial match case. It feels like a new operator. There should be a declarative way to define that as partial matches won't likely make sense as a default behavior due to how matching is defined and altering that would be a breaking change.

[just-jeb]

Breaking change is not a big deal if you release a new major version 😄
I realize that this is now a generic library, but I think that the vast majority of your users is the webpack users and loaders merging scenario is a perfectly valid one.

In particular this one works:

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

   const expected = {
        "module": {
          "rules": [
            {
              "test": "/\\.scss$|\\.sass$/",
              "use": [
                {
                  "loader": "sass-loader",
                  "options": {
                    "sourceMap": true,
                    "sassOptions": {
                      "precision": 8,
                      "outputStyle": "expanded"
                    }
                  }
                },
                {
                  "loader": "sass-resources-loader",
                  "options": {
                    "resources": [
                      "src/styles/includes.scss"
                    ]
                  }
                }
              ]
            }
          ]
        }
      }

Note that the first configuration doesn't have sass-resources-loader while the second does.

However, the moment I remove sass-loader from the second configuration it breaks:

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

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

I wouldn't expect that, I'd expect it to work just like the first example - if certain field doesn't exist then it's "merged" vacuously

Both examples use this rule set:

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

So if one day I had to merge the configs like in first example, and the day after I didn't need the modification of sass-loader anymore and removed it, the code would break, because mergeWithRules does not support vacuous cases.
WDYT?

[bebraw]

Yeah, that's a good example I can use for a test. It definitely should fall back to a default merge if rules don't match.

[just-jeb]

Do you think you'd be able to provide a fix soon?

[bebraw]

Do you think you'd be able to provide a fix soon?

I will give it a quick go. If it doesn't require a big rewrite, then I can publish the fix soon but if it requires an architectural change, then it's going to be way harder.

[bebraw]

@just-jeb I have initial fix at #168. Feel free to comment there. I'll merge and publish after that.