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

Modify 'import css' order more than 2 times, it will not work #406

Closed
shield2018 opened this issue May 30, 2019 · 14 comments
Closed

Modify 'import css' order more than 2 times, it will not work #406

shield2018 opened this issue May 30, 2019 · 14 comments

Comments

@shield2018
Copy link

  • Operating System:mac os 10.14.4
  • Node Version:8.9.0
  • NPM Version:6.8.0
  • webpack Version:4.16.5
  • mini-css-extract-plugin Version:0.7.0

Expected Behavior

when I use webpack-dev-server. I modify css order. The out css should be modified at the same time.
For example:

import 'a.less';
import 'b.less';

out:

.a{
}
.b{
}

modify:

import 'b.less';
import 'a.less';

out:

.b{
}
.a{
}

Actual Behavior

Only the first modification works. Don't work from the second time.
For example:

import 'a.less';
import 'b.less';

out:

.a{
}
.b{
}

modify:

import 'b.less';
import 'a.less';

out:

.b{
}
.a{
}

modify:

import 'a.less';
import 'b.less';

out:

.b{
}
.a{
}

not change

How Do We Reproduce?

Modify import more than 2 times

@shield2018 shield2018 changed the title Modify css order not work Modify 'import css' order more than 2 times, it will not work May 30, 2019
@shield2018
Copy link
Author

this is because CssModule does not have 'libIdent' method.

@alexander-akait
Copy link
Member

Please create minimum reproducible test repo, thanks

@alexander-akait
Copy link
Member

Looks like problem in less-loader

@shield2018

This comment has been minimized.

@shield2018

This comment has been minimized.

@shield2018
Copy link
Author

node 12.3.1 result is right

@alexander-akait
Copy link
Member

Please create minimum reproducible test repo

@shield2018
Copy link
Author

@alexander-akait
Copy link
Member

Thanks looks like bug

@sokra
Copy link
Member

sokra commented Jun 12, 2019

I think this was already fixed by webpack/webpack#8243

Could you try upgrading to the latest webpack version?

@sokra
Copy link
Member

sokra commented Jun 12, 2019

No doesn't seem to be fixed

@sokra
Copy link
Member

sokra commented Jun 12, 2019

Ok I found the issue.

While the correct order is determined when emitting the asset, calculating the content hash is not done in with the correct order:

for (const m of chunk.modulesIterable) {
if (m.type === MODULE_TYPE) {
m.updateHash(hash);
}
}

I was able to fix the problem after changing this to:

        const modules = Array.from(chunk.modulesIterable).filter(module => module.type === MODULE_TYPE);
        const orderedModules = this.determineModulesInOrder(chunk, modules);

        for (const m of orderedModules) {
          if (m.type === MODULE_TYPE) {
            m.updateHash(hash);
          }
        }

I also extracted the following code snippet into a new function determineModulesInOrder(chunk, modules, compilation, requestShortener):

let usedModules;
const [chunkGroup] = chunk.groupsIterable;
if (typeof chunkGroup.getModuleIndex2 === 'function') {
// Store dependencies for modules
const moduleDependencies = new Map(modules.map((m) => [m, new Set()]));
// Get ordered list of modules per chunk group
// This loop also gathers dependencies from the ordered lists
// Lists are in reverse order to allow to use Array.pop()
const modulesByChunkGroup = Array.from(chunk.groupsIterable, (cg) => {
const sortedModules = modules
.map((m) => {
return {
module: m,
index: cg.getModuleIndex2(m),
};
})
// eslint-disable-next-line no-undefined
.filter((item) => item.index !== undefined)
.sort((a, b) => b.index - a.index)
.map((item) => item.module);
for (let i = 0; i < sortedModules.length; i++) {
const set = moduleDependencies.get(sortedModules[i]);
for (let j = i + 1; j < sortedModules.length; j++) {
set.add(sortedModules[j]);
}
}
return sortedModules;
});
// set with already included modules in correct order
usedModules = new Set();
const unusedModulesFilter = (m) => !usedModules.has(m);
while (usedModules.size < modules.length) {
let success = false;
let bestMatch;
let bestMatchDeps;
// get first module where dependencies are fulfilled
for (const list of modulesByChunkGroup) {
// skip and remove already added modules
while (list.length > 0 && usedModules.has(list[list.length - 1])) {
list.pop();
}
// skip empty lists
if (list.length !== 0) {
const module = list[list.length - 1];
const deps = moduleDependencies.get(module);
// determine dependencies that are not yet included
const failedDeps = Array.from(deps).filter(unusedModulesFilter);
// store best match for fallback behavior
if (!bestMatchDeps || bestMatchDeps.length > failedDeps.length) {
bestMatch = list;
bestMatchDeps = failedDeps;
}
if (failedDeps.length === 0) {
// use this module and remove it from list
usedModules.add(list.pop());
success = true;
break;
}
}
}
if (!success) {
// no module found => there is a conflict
// use list with fewest failed deps
// and emit a warning
const fallbackModule = bestMatch.pop();
compilation.warnings.push(
new Error(
`chunk ${chunk.name || chunk.id} [${pluginName}]\n` +
'Conflicting order between:\n' +
` * ${fallbackModule.readableIdentifier(requestShortener)}\n` +
`${bestMatchDeps
.map((m) => ` * ${m.readableIdentifier(requestShortener)}`)
.join('\n')}`
)
);
usedModules.add(fallbackModule);
}
}
} else {
// fallback for older webpack versions
// (to avoid a breaking change)
// TODO remove this in next mayor version
// and increase minimum webpack version to 4.12.0
modules.sort((a, b) => a.index2 - b.index2);
usedModules = modules;
}

To avoid emitting warnings in the hashing part I wrapped

compilation.warnings.push(
new Error(
`chunk ${chunk.name || chunk.id} [${pluginName}]\n` +
'Conflicting order between:\n' +
` * ${fallbackModule.readableIdentifier(requestShortener)}\n` +
`${bestMatchDeps
.map((m) => ` * ${m.readableIdentifier(requestShortener)}`)
.join('\n')}`
)
);

in if(compilation) {}


There are a bunch of other problems:

this.id = ''

This doesn't work as well as expected for multiple reasons:

The NamedModulesPlugin kind of ignores this while trying to eliminate duplicate ids.

ids are cleared when recompiling, which resets this on incremental compilation.

In Chunks Modules are sorted by id, so this kind of affects the order.

As hack we could use this instead:

  get id() {
    return '';
  }

  set id(value) {}

webpack 5 changes a lot of this stuff. I. e. there is no id property on Modules anymore and ID assigning is more reliable when modules already have ids.


Send a PR

@shield2018
Copy link
Author

for (const m of chunk.modulesIterable) {
if (m.type === MODULE_TYPE) {
m.updateHash(hash);
}
}

changing this to:

const modules = Array.from(chunk.modulesIterable).filter(module => module.type === MODULE_TYPE);
modules.sort((a,b)=>{
  return a.index-b.index;
})
for (const m of modules) {
  if (m.type === MODULE_TYPE) {
    m.updateHash(hash);
  }
}

@alexander-akait
Copy link
Member

Fixed for webpack v5 and v1.3.8

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

No branches or pull requests

3 participants