Skip to content

Commit df79602

Browse files
evilebottnawievilebottnawi
authoredJan 10, 2020
fix: do not duplicate css on composes (#1040)
·
v7.1.2v3.4.2
1 parent 7c9f47b commit df79602

13 files changed

+847
-638
lines changed
 

‎package-lock.json

+612-540
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎package.json

+14-14
Original file line numberDiff line numberDiff line change
@@ -57,38 +57,38 @@
5757
"schema-utils": "^2.6.0"
5858
},
5959
"devDependencies": {
60-
"@babel/cli": "^7.7.4",
61-
"@babel/core": "^7.7.4",
62-
"@babel/preset-env": "^7.7.4",
63-
"@commitlint/cli": "^8.2.0",
64-
"@commitlint/config-conventional": "^8.2.0",
60+
"@babel/cli": "^7.7.7",
61+
"@babel/core": "^7.7.7",
62+
"@babel/preset-env": "^7.7.7",
63+
"@commitlint/cli": "^8.3.4",
64+
"@commitlint/config-conventional": "^8.3.4",
6565
"@webpack-contrib/defaults": "^6.3.0",
6666
"@webpack-contrib/eslint-config-webpack": "^3.0.0",
6767
"babel-jest": "^24.9.0",
68-
"commitlint-azure-pipelines-cli": "^1.0.2",
68+
"commitlint-azure-pipelines-cli": "^1.0.3",
6969
"cross-env": "^6.0.3",
7070
"del": "^5.1.0",
7171
"del-cli": "^3.0.0",
7272
"es-check": "^5.1.0",
73-
"eslint": "^6.7.1",
74-
"eslint-config-prettier": "^6.7.0",
75-
"eslint-plugin-import": "^2.18.2",
73+
"eslint": "^6.8.0",
74+
"eslint-config-prettier": "^6.9.0",
75+
"eslint-plugin-import": "^2.19.1",
7676
"file-loader": "^5.0.2",
77-
"husky": "^3.1.0",
77+
"husky": "^4.0.6",
7878
"jest": "^24.9.0",
7979
"jest-junit": "^10.0.0",
8080
"lint-staged": "^9.5.0",
81-
"memfs": "^3.0.1",
81+
"memfs": "^3.0.3",
8282
"npm-run-all": "^4.1.5",
8383
"postcss-loader": "^3.0.0",
8484
"postcss-preset-env": "^6.7.0",
8585
"prettier": "^1.19.1",
86-
"sass": "^1.23.7",
87-
"sass-loader": "^8.0.0",
86+
"sass": "^1.24.4",
87+
"sass-loader": "^8.0.1",
8888
"standard-version": "^7.0.1",
8989
"strip-ansi": "^6.0.0",
9090
"url-loader": "^3.0.0",
91-
"webpack": "^4.41.3"
91+
"webpack": "^4.41.5"
9292
},
9393
"keywords": [
9494
"webpack",

‎src/runtime/api.js

+19-1
Original file line numberDiff line numberDiff line change
@@ -22,15 +22,33 @@ module.exports = function(useSourceMap) {
2222

2323
// import a list of modules into the list
2424
// eslint-disable-next-line func-names
25-
list.i = function(modules, mediaQuery) {
25+
list.i = function(modules, mediaQuery, dedupe) {
2626
if (typeof modules === 'string') {
2727
// eslint-disable-next-line no-param-reassign
2828
modules = [[null, modules, '']];
2929
}
3030

31+
const alreadyImportedModules = {};
32+
33+
if (dedupe) {
34+
for (let i = 0; i < this.length; i++) {
35+
// eslint-disable-next-line prefer-destructuring
36+
const id = this[i][0];
37+
38+
if (id != null) {
39+
alreadyImportedModules[id] = true;
40+
}
41+
}
42+
}
43+
3144
for (let i = 0; i < modules.length; i++) {
3245
const item = [].concat(modules[i]);
3346

47+
if (dedupe && alreadyImportedModules[item[0]]) {
48+
// eslint-disable-next-line no-continue
49+
continue;
50+
}
51+
3452
if (mediaQuery) {
3553
if (!item[2]) {
3654
item[2] = mediaQuery;

‎src/utils.js

+2-2
Original file line numberDiff line numberDiff line change
@@ -329,7 +329,7 @@ function getImportCode(
329329
case 'icss-import':
330330
{
331331
const { importName, url, media } = item;
332-
const preparedMedia = media ? `, ${JSON.stringify(media)}` : '';
332+
const preparedMedia = media ? `, ${JSON.stringify(media)}` : ', ""';
333333

334334
if (!importPrefix) {
335335
importPrefix = getImportPrefix(loaderContext, importLoaders);
@@ -348,7 +348,7 @@ function getImportCode(
348348
);
349349

350350
if (exportType === 'full') {
351-
codeItems.push(`exports.i(${importName}${preparedMedia});`);
351+
codeItems.push(`exports.i(${importName}${preparedMedia}, true);`);
352352
}
353353
}
354354
break;

‎test/__snapshots__/icss.test.js.snap

+2-2
Original file line numberDiff line numberDiff line change
@@ -176,7 +176,7 @@ exports[`ICSS show work with the case "import": module 1`] = `
176176
var ___CSS_LOADER_API_IMPORT___ = require(\\"../../../../../src/runtime/api.js\\");
177177
var ___CSS_LOADER_ICSS_IMPORT_0___ = require(\\"-!../../../../../src/index.js??[ident]!./vars.css\\");
178178
exports = ___CSS_LOADER_API_IMPORT___(false);
179-
exports.i(___CSS_LOADER_ICSS_IMPORT_0___);
179+
exports.i(___CSS_LOADER_ICSS_IMPORT_0___, \\"\\", true);
180180
// Module
181181
exports.push([module.id, \\".className {\\\\n color: \\" + ___CSS_LOADER_ICSS_IMPORT_0___.locals[\\"primary-color\\"] + \\";\\\\n}\\\\n\\", \\"\\"]);
182182
// Exports
@@ -215,7 +215,7 @@ exports[`ICSS show work with the case "import-reserved-keywords": module 1`] = `
215215
var ___CSS_LOADER_API_IMPORT___ = require(\\"../../../../../src/runtime/api.js\\");
216216
var ___CSS_LOADER_ICSS_IMPORT_0___ = require(\\"-!../../../../../src/index.js??[ident]!./vars.css\\");
217217
exports = ___CSS_LOADER_API_IMPORT___(false);
218-
exports.i(___CSS_LOADER_ICSS_IMPORT_0___);
218+
exports.i(___CSS_LOADER_ICSS_IMPORT_0___, \\"\\", true);
219219
// Module
220220
exports.push([module.id, \\".className {\\\\n color: \\" + ___CSS_LOADER_ICSS_IMPORT_0___.locals[\\"primary-color\\"] + \\";\\\\n display: \\" + ___CSS_LOADER_ICSS_IMPORT_0___.locals[\\"secondary-color\\"] + \\";\\\\n}\\\\n\\", \\"\\"]);
221221
// Exports

‎test/__snapshots__/modules-option.test.js.snap

+145-74
Large diffs are not rendered by default.

‎test/fixtures/modules/dedupe/buttons/button.css

+6
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
.button
2+
{
3+
border:none;
4+
padding:7px 15px;
5+
cursor:pointer;
6+
}

‎test/fixtures/modules/dedupe/buttons/primary-button.css

+6
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
.primaryButton
2+
{
3+
composes:button from './button.css';
4+
background-color:blue;
5+
color:white;
6+
}

‎test/fixtures/modules/dedupe/buttons/secondary-button.css

+6
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
.secondaryButton
2+
{
3+
composes:button from './button.css';
4+
background-color:#555;
5+
color:white;
6+
}

‎test/fixtures/modules/dedupe/source.css

+9
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
.nextButton
2+
{
3+
composes:primaryButton from './buttons/primary-button.css';
4+
}
5+
6+
.backButton
7+
{
8+
composes:secondaryButton from './buttons/secondary-button.css';
9+
}

‎test/fixtures/modules/dedupe/source.js

+5
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
import css from './source.css';
2+
3+
__export__ = css;
4+
5+
export default css;

‎test/modules-option.test.js

+16
Original file line numberDiff line numberDiff line change
@@ -549,4 +549,20 @@ describe('"modules" option', () => {
549549
expect(getWarnings(stats)).toMatchSnapshot('warnings');
550550
expect(getErrors(stats)).toMatchSnapshot('errors');
551551
});
552+
553+
it('should dedupe same modules in one module (issue #1037)', async () => {
554+
const compiler = getCompiler('./modules/dedupe/source.js', {
555+
modules: true,
556+
});
557+
const stats = await compile(compiler);
558+
559+
expect(
560+
getModuleSource('./modules/dedupe/source.css', stats)
561+
).toMatchSnapshot('module');
562+
expect(getExecutedCode('main.bundle.js', compiler, stats)).toMatchSnapshot(
563+
'result'
564+
);
565+
expect(getWarnings(stats)).toMatchSnapshot('warnings');
566+
expect(getErrors(stats)).toMatchSnapshot('errors');
567+
});
552568
});

‎test/runtime/api.test.js

+5-5
Original file line numberDiff line numberDiff line change
@@ -101,9 +101,7 @@ describe('api', () => {
101101
expect(m.toString()).toMatchSnapshot();
102102
});
103103

104-
it('should toString without source mapping if btoa not available', () => {
105-
global.btoa = null;
106-
104+
it('should toString with a source map without "sourceRoot"', () => {
107105
const m = api(true);
108106

109107
m.push([
@@ -114,14 +112,15 @@ describe('api', () => {
114112
file: 'test.scss',
115113
sources: ['./path/to/test.scss'],
116114
mappings: 'AAAA;',
117-
sourceRoot: 'webpack://',
118115
},
119116
]);
120117

121118
expect(m.toString()).toMatchSnapshot();
122119
});
123120

124-
it.only('should toString with a source map without "sourceRoot"', () => {
121+
it('should toString without source mapping if btoa not available', () => {
122+
global.btoa = null;
123+
125124
const m = api(true);
126125

127126
m.push([
@@ -132,6 +131,7 @@ describe('api', () => {
132131
file: 'test.scss',
133132
sources: ['./path/to/test.scss'],
134133
mappings: 'AAAA;',
134+
sourceRoot: 'webpack://',
135135
},
136136
]);
137137

0 commit comments

Comments
 (0)
Please sign in to comment.