feat: add option.base

[ilanc]

…loader/issues/163

What kind of change does this PR introduce?
bugfix / workaround for #163

Did you add tests for your changes?
no

If relevant, did you update the README?
no

Summary
This is a workaround. It allows the user to manually specify base id's for each bundle when multiple bundles are used on a page. The user would for have to manually specify the order in various webpack.configs like so:
dll1.js
loaders:[ { test: /\.css$/, loader: 'style-loader!css-loader' } ]
dll2.js
loaders:[ { test: /\.css$/, loader: 'style-loader?cssBase=1000!css-loader' } ]
app.js
loaders:[ { test: /\.css$/, loader: 'style-loader?cssBase=2000!css-loader' } ]

see #163

Does this PR introduce a breaking change?
no - the user has to opt in - there's no impact if the user doesn't specify option.cssBase

Other information

[jsf-clabot]

All committers have signed the CLA.

[zojeda]

+1 we are having the same issue, this worked fine for us, hope this PR gets approved

[michael-ciniawsky]

@ilanc Please close and reopen the PR to trigger the CLA Bot again and signed the CLA please 😛

[michael-ciniawsky]

Also add a test please

[ilanc]

Hi there @michael-ciniawsky I've re-signed the CLA. How do I add a test in this case? Did you see the related bug report btw? https://github.com/ilanc/style-loader-bug

[ekulabuhov]

Please add an explanation of the new option to the README.

[ekulabuhov]

@ilanc a good unit test here would be to re-create the exact problem you had and show that your fix indeed solves it. Have you looked at the test/basicTest.js? They exercise webpack with different loader setups and configurations in memory. Tell me if you're having any problems setting it up.

[ilanc]

Thanks @ekulabuhov I've just gone on holiday I'll look into it in 10 days time

[ilanc]

Hi chaps, I've added the notes to the README. I don't have the tests in my branch - looks like these were added in Feb. Do you want me to merge (or perhaps rebase) my changes off the latest head?

[bebraw]

@ilanc Please rebase. A lot has been going on. 😄

[michael-ciniawsky]

@ilanc I will try to take a look at it, but I haven't much experience using the DllPlugin

@bebraw @d3viant0ne @evilebottnawi anyone used to DllPlugin and cpuld help @ilanc with this? 🙃

[alexander-akait]

@michael-ciniawsky i have pure knowledge about Dll 😄 /cc @ilanc i am right that https://github.com/ilanc/style-loader-bug repo contain example bug?

[ilanc]

@michael-ciniawsky @bebraw @d3viant0ne @evilebottnawi

Yes https://github.com/ilanc/style-loader-bug gives you a good example of DllPlugin in use. It's theoretical though - i.e. it's not a real app. See this article if you want to see a real use case.

The style-loader-bug repo is a good minimal example for use as a unit test. However as I mentioned you're going to have to jump through a lot of hoops if you want to have a unit test in the style-loader project which depends on functionality in the core webpack repo (DllPlugin functionality lives in core webpack IIRC).

CssBase is only useful with DllPlugins - there's no css clash between existingStyles and webpack css - I thought of trying this as a unit test but there's no clash so it's not a useful unit test for cssBase. The clash only occurs when you load a DllPlugin.

Load up the style-loader-bug repo and follow the build instructions in the readme.md then run in chrome with dev tools open and single step through the code. There are comments in the .html file showing what's going wrong.

[michael-ciniawsky]

cssBase doesn't introduce any possible side effects for other users when not using/setting it right (I need to ask so naively 🙃 )? If so and it works with DllPlugin in a sample project, we could leave the test(s) for it in style-loader out, if impossible to unit test atm. Although I intend to run tests through webpack in the near future, but that's not a concern of yours, since it isn't in place yet 😛

[michael-ciniawsky]

- var id = item[0] + (+(options.cssBase || 0))
+ var id = options.cssBase ? item[0] + options.cssBase : item[0]

[michael-ciniawsky]

Please use webpack 2 syntax here 😛

{
  test: /\.css$/,
  use: [ 
    { loader: 'style-loader', options: { cssBase: 1000 } },
    'css-loader'
  ]
}

[michael-ciniawsky]

@ilanc Could we use e.g options.cssBase * 1e3 to make the options terser

- { loader: 'style-loader`, options: { cssBase: 1000 } }
+ { loader: 'style-loader`, options: { cssBase: 1 } }

Naming

cssBase => base
cssBase => idBase

The intention to bump the module id range by e.g n * 1e3 when using DllPlugin isn't fully clear imho :)

[ilanc]

I don't think options.cssBase * 1e3 makes any meaningful difference - i arbitrarily choose 1000. If you only require 5 css files in Dll1 then Dll2 probably only needs a cssBase of 6* or higher in order to avoid clashing with Dll1's css modules.

I prefer cssBase over the other suggestions because it's a css specific problem - the others are a bit generic.

* Of course the number of modules in a Dll plugin isn't exactly equal to the number of require statements so using 1000 is just being cautious.

[michael-ciniawsky]

I don't think options.cssBase * 1e3 makes any meaningful difference - i arbitrarily choose 1000. If you only require 5 css files in Dll1 then Dll2 probably only needs a cssBase of 6* or higher in order to avoid clashing with Dll1's css modules.

👍

I prefer cssBase over the other suggestions because it's a css specific problem - the others are a bit generic.

Disagreeing here since the css prefix doesn't add any meaning imho (nothing else then CSS is 'processed' by this loader anyways), so we can keep it terse and generic. Please rename to either id || base || range with base as the favorite, but enterily free choice between those three options and good to fo then 😛

[michael-ciniawsky]

Please also close and reopen the PR to trigger the CLA Bot again and sign the CLA

[ilanc]

@michael-ciniawsky cool I think it's ready to go - looks like the CLA bot did it's thing.

[michael-ciniawsky]

@ilanc Thx

[ilanc]

:) happy to help