Skip to content

feat: improve warning of conflict order #465

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

Merged
merged 3 commits into from
Nov 22, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4,020 changes: 2,384 additions & 1,636 deletions package-lock.json

Large diffs are not rendered by default.

49 changes: 34 additions & 15 deletions src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -90,12 +90,7 @@ class CssModule extends webpack.Module {
}

class CssModuleFactory {
create(
{
dependencies: [dependency],
},
callback
) {
create({ dependencies: [dependency] }, callback) {
callback(null, new CssModule(dependency));
}
}
Expand Down Expand Up @@ -416,6 +411,9 @@ class MiniCssExtractPlugin {
if (typeof chunkGroup.getModuleIndex2 === 'function') {
// Store dependencies for modules
const moduleDependencies = new Map(modules.map((m) => [m, new Set()]));
const moduleDependenciesReasons = new Map(
modules.map((m) => [m, new Map()])
);

// Get ordered list of modules per chunk group
// This loop also gathers dependencies from the ordered lists
Expand All @@ -435,9 +433,14 @@ class MiniCssExtractPlugin {

for (let i = 0; i < sortedModules.length; i++) {
const set = moduleDependencies.get(sortedModules[i]);
const reasons = moduleDependenciesReasons.get(sortedModules[i]);

for (let j = i + 1; j < sortedModules.length; j++) {
set.add(sortedModules[j]);
const module = sortedModules[j];
set.add(module);
const reason = reasons.get(module) || new Set();
reason.add(cg);
reasons.set(module, reason);
}
}

Expand Down Expand Up @@ -489,16 +492,32 @@ class MiniCssExtractPlugin {
// and emit a warning
const fallbackModule = bestMatch.pop();
if (!this.options.ignoreOrder) {
const reasons = moduleDependenciesReasons.get(fallbackModule);
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')}`
[
`chunk ${chunk.name || chunk.id} [${pluginName}]`,
'Conflicting order. Following module has been added:',
` * ${fallbackModule.readableIdentifier(requestShortener)}`,
'despite it was not able to fulfill desired ordering with these modules:',
...bestMatchDeps.map((m) => {
const goodReasonsMap = moduleDependenciesReasons.get(m);
const goodReasons =
goodReasonsMap && goodReasonsMap.get(fallbackModule);
const failedChunkGroups = Array.from(
reasons.get(m),
(cg) => cg.name
).join(', ');
const goodChunkGroups =
goodReasons &&
Array.from(goodReasons, (cg) => cg.name).join(', ');
return [
` * ${m.readableIdentifier(requestShortener)}`,
` - couldn't fulfill desired order of chunk group(s) ${failedChunkGroups}`,
` - while fulfilling desired order of chunk group(s) ${goodChunkGroups}`,
].join('\n');
}),
].join('\n')
)
);
}
Expand Down
14 changes: 14 additions & 0 deletions test/TestCases.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,13 @@ function compareDirectory(actual, expected) {
}
}

function compareWarning(actual, expectedFile) {
if (!fs.existsSync(expectedFile)) return;

const expected = require(expectedFile); // eslint-disable-line global-require,import/no-dynamic-require
expect(actual).toBe(expected);
}

describe('TestCases', () => {
const casesDirectory = path.resolve(__dirname, 'cases');
const outputDirectory = path.resolve(__dirname, 'js');
Expand Down Expand Up @@ -93,6 +100,13 @@ describe('TestCases', () => {

compareDirectory(outputDirectoryForCase, expectedDirectory);

const expectedWarning = path.resolve(directoryForCase, 'warnings.js');
const actualWarning = stats.toString({
all: false,
warnings: true,
});
compareWarning(actualWarning, expectedWarning);

done();
});
}, 10000);
Expand Down
3 changes: 3 additions & 0 deletions test/cases/ignoreOrderFalse/e3.css
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
body {
content: 'e3';
}
4 changes: 4 additions & 0 deletions test/cases/ignoreOrderFalse/expected/styles.css
Original file line number Diff line number Diff line change
Expand Up @@ -6,3 +6,7 @@ body {
content: 'e1';
}

body {
content: 'e3';
}

2 changes: 2 additions & 0 deletions test/cases/ignoreOrderFalse/index3.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
import './e2.css';
import './e3.css';
12 changes: 12 additions & 0 deletions test/cases/ignoreOrderFalse/warnings.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
const cssLoaderPath = require.resolve('css-loader').replace(/\\/g, '/');

module.exports = [
'',
'WARNING in chunk styles [mini-css-extract-plugin]',
'Conflicting order. Following module has been added:',
` * css ${cssLoaderPath}!./e2.css`,
'despite it was not able to fulfill desired ordering with these modules:',
` * css ${cssLoaderPath}!./e1.css`,
" - couldn't fulfill desired order of chunk group(s) entry2",
' - while fulfilling desired order of chunk group(s) entry1',
].join('\n');
1 change: 1 addition & 0 deletions test/cases/ignoreOrderFalse/webpack.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ module.exports = {
entry: {
entry1: './index.js',
entry2: './index2.js',
entry3: './index3.js',
},
module: {
rules: [
Expand Down