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

A loader with exclude function can not merge ideally #75

Closed
liangklfangl opened this issue Mar 26, 2017 · 5 comments · Fixed by #135
Closed

A loader with exclude function can not merge ideally #75

liangklfangl opened this issue Mar 26, 2017 · 5 comments · Fixed by #135
Milestone

Comments

@liangklfangl
Copy link

liangklfangl commented Mar 26, 2017

Bellow is an example i encountered of webpack2, i hope someone can give me some suggestions:

const loader1= {
	module:{
       rules:[{
          test: /\.jsx$/,
	      use:[{
	      	loader: 'babel-loader',
	      	 exclude: function(path){
               var isNpmModule=!!path.match(/node_modules/);
               return isNpmModule;
            },
	      	options:{
	      	}
	      }]
	   }]
	}
}
//loader will override it!
const loader2 = {
	module:{
       rules:[
       {
          test: /\.jsx$/,
	      use:[{
	      	loader: 'after',
	      	 exclude: function(path){
               var isNpmModule=!!path.match(/node_modules/);
               return isNpmModule;
            },
	      	options:{}
	      }]
	      
	  }]
	}
}
const defaultWebpackConfig = merge(loader1,loader2);
 const webpackRules = merge.smart({
          rules: loader1.module.rules
        }, {
          rules:loader2.module.rules
        });
   defaultWebpackConfig.module.rules = webpackRules.rules;

Actually, i want second loader will override previous one, but actually i get one as bellow:

 [ { test: { /\.jsx$/ [lastIndex]: 0 },
    use:
     [ { loader: 'babel-loader',
         exclude:
          { [Function: exclude]
            [length]: 1,
            [name]: 'exclude',
            [arguments]: null,
            [caller]: null,
            [prototype]: [Object] },
         options: {} },
       { loader: 'after',
         exclude:
          { [Function: exclude]
            [length]: 1,
            [name]: 'exclude',
            [arguments]: null,
            [caller]: null,
            [prototype]: [Object] },
         options: {} },
       [length]: 2 ] },
  [length]: 1 ]
@bebraw
Copy link
Member

bebraw commented Mar 26, 2017

Please see merge.smartStrategy. You can override the default behavior there for more control. Going with override in this particular case would break basic assumptions I think.

@liangklfangl
Copy link
Author

Firstly, i have to apologize for what i said above, here, i will show this issue correctly:

const loader1= {
	module:{
       rules:[{
          test: /\.jsx$/,
          exclude: function(path){
               var isNpmModule=!!path.match(/node_modules/);
               return isNpmModule;
            },
	      use:[{
	      	loader: 'babel-loader',
	      	options:{
	      	}
	      }]
	   }]
	}
}
const loader2 = {
	module:{
       rules:[
       {
          test: /\.jsx$/,
          exclude: function(path){
               var isNpmModule=!!path.match(/node_modules/);
               return isNpmModule;
            },
	      use:[{
	       	loader: 'after',
	      	options:{}
	      }]
	  }]
	}
}
const defaultWebpackConfig = merge(loader1,loader2);
 const webpackRules = merge.smart({
          rules: loader1.module.rules
        }, {
          rules:loader2.module.rules
        });
   defaultWebpackConfig.module.rules = webpackRules.rules;

Actually, i want to combine two rules into one while actually i still get an object as bellow:

 [ { test: { /\.jsx$/ [lastIndex]: 0 },
    exclude:
     { [Function: exclude]
       [length]: 1,
       [name]: 'exclude',
       [arguments]: null,
       [caller]: null,
       [prototype]: exclude { [constructor]: [Circular] } },
    use: [ { loader: 'babel-loader', options: {} }, [length]: 1 ] },
  { test: { /\.jsx$/ [lastIndex]: 0 },
    exclude:
     { [Function: exclude]
       [length]: 1,
       [name]: 'exclude',
       [arguments]: null,
       [caller]: null,
       [prototype]: exclude { [constructor]: [Circular] } },
    use: [ { loader: 'after', options: {} }, [length]: 1 ] },
  [length]: 2 ]

You can see above that there are two rule test. But, if you replace function with a string, for example:

exclude:'node_modules'

You will get what you want(Only one test rule leaved):

 [ { test: { /\.jsx$/ [lastIndex]: 0 },
    exclude: 'node_modules',
    use:
     [ { loader: 'babel-loader', options: {} },
       { loader: 'after', options: {} },
       [length]: 2 ] },
  [length]: 1 ]

@bebraw
Copy link
Member

bebraw commented Mar 26, 2017

Ah, yeah. I think the problem is that the current code doesn't have any specific support for functions yet. It would probably have to run possible functions to see how they evaluate to see if they give the same result so the rules can be merged. The problem is that webpack controls path and there's no way to tell what it's going to be outside of webpack.

Maybe the best option would be to document this as there's no way to "fix" it in webpack-merge.

@chinesedfan
Copy link
Contributor

0.20.0 / 2016-11-27
===================

  • Feature: Add support for merging functions. This feature has been designed postcss in mind. It executes the functions, picks their results, and packs them again.

It is interesting that function merging has been implemented in v0.20.0. And I tested locally, v4.2.2, only got one rule test.

// console.log(JSON.stringify(mergedConfig,null,2))
{
  "0": {
    "test": {},
    "use": [
      {
        "loader": "babel-loader",
        "options": {}
      },
      {
        "loader": "after",
        "options": {}
      }
    ]
  }
}

@bebraw bebraw added this to the v5 milestone Jul 3, 2020
@bebraw
Copy link
Member

bebraw commented Jul 3, 2020

I'll drop support for merge.smart in v5 as it's tricky to support.It's best if you can refactor the configuration to regular merge. See #135 for progress.

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

Successfully merging a pull request may close this issue.

3 participants