Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Using mergeWithRules with top level entries #159

Closed
just-jeb opened this issue Nov 19, 2020 · 3 comments
Closed

Using mergeWithRules with top level entries #159

just-jeb opened this issue Nov 19, 2020 · 3 comments

Comments

@just-jeb
Copy link
Contributor

just-jeb commented Nov 19, 2020

I suspect it's bigger than just top level entries, most probably it also relates to anything that is a primitive.
I'd like to merge two configs with loaders and externals.
As per documentation in order to properly merge loaders I have to use mergeWithRules and indeed the loaders part works perfect.
The problem is with externals, seems like it's completely ignored and used with the default strategy (append).

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

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

This test will fail, because externals are not replaced.
Is this a bug or there is another way to merge both loaders and other "primitive" properties?

Node: 12.18.2
OS: macOS 10.15.6

@bebraw
Copy link
Member

bebraw commented Nov 19, 2020

I think #151 is exactly about this problem. The problem is that replace doesn't do anything without a match.

I've prototyped a solution at #152. It would be great if you could have a quick look and let me think so I can finalize that.

@just-jeb
Copy link
Contributor Author

Looks like it is. I commented on the PR. Overall I don't think it's the best way to go - it overcomplicates the configuration and creates confusion.
I think the only should be derived from the context rather than explicitly specified.

@bebraw
Copy link
Member

bebraw commented Nov 19, 2020

I think the only should be derived from the context rather than explicitly specified.

Yup, it's a good way. Thanks for the feedback. I'll add some automation in the next pass. 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants