fix: algorithm for importing modules (#449)

evilebottnawi · web-flow · commit 91ceaf2b7e03 · 2019-12-25T15:58:25.000+03:00
diff --git a/src/index.js b/src/index.js
@@ -161,9 +161,7 @@ if (content.locals) {
 
 exported.use = function() {
   if (!(refs++)) {
-    var id = ${loaderUtils.stringifyRequest(this, `!!${request}`)};
-
-    dispose = api(id, content, options);
+    dispose = api(module.id, content, options);
   }
 
   return exported;
@@ -251,8 +249,7 @@ var options = ${JSON.stringify(options)};
 options.insert = ${insert};
 options.singleton = ${isSingleton};
 
-var id = ${loaderUtils.stringifyRequest(this, `!!${request}`)};
-var update = api(id, content, options);
+var update = api(module.id, content, options);
 
 var exported = content.locals ? content.locals : {};
 
diff --git a/src/runtime/injectStylesIntoStyleTag.js b/src/runtime/injectStylesIntoStyleTag.js
@@ -1,5 +1,3 @@
-const stylesInDom = {};
-
 const isOldIE = (function isOldIE() {
   let memo;
 
@@ -46,38 +44,22 @@ const getTarget = (function getTarget() {
   };
 })();
 
-function addModulesToDom(id, list, options) {
-  if (Object.prototype.toString.call(list) !== '[object Array]') {
-    return;
-  }
-
-  id = options.base ? id + options.base : id;
-
-  if (!stylesInDom[id]) {
-    stylesInDom[id] = [];
-  }
+const stylesInDom = {};
 
+function modulesToDom(moduleId, list, options) {
   for (let i = 0; i < list.length; i++) {
-    const item = list[i];
-    const part = { css: item[1], media: item[2], sourceMap: item[3] };
-    const styleInDomById = stylesInDom[id];
+    const part = {
+      css: list[i][1],
+      media: list[i][2],
+      sourceMap: list[i][3],
+    };
 
-    if (styleInDomById[i]) {
-      styleInDomById[i].updater(part);
+    if (stylesInDom[moduleId][i]) {
+      stylesInDom[moduleId][i](part);
     } else {
-      styleInDomById.push({ updater: addStyle(part, options) });
+      stylesInDom[moduleId].push(addStyle(part, options));
     }
   }
-
-  for (let j = list.length; j < stylesInDom[id].length; j++) {
-    stylesInDom[id][j].updater();
-  }
-
-  stylesInDom[id].length = list.length;
-
-  if (stylesInDom[id].length === 0) {
-    delete stylesInDom[id];
-  }
 }
 
 function insertStyleElement(options) {
@@ -230,7 +212,7 @@ function addStyle(obj, options) {
   };
 }
 
-module.exports = (id, list, options) => {
+module.exports = (moduleId, list, options) => {
   options = options || {};
 
   // Force single-tag solution on IE6-9, which has a hard limit on the # of <style>
@@ -239,9 +221,37 @@ module.exports = (id, list, options) => {
     options.singleton = isOldIE();
   }
 
-  addModulesToDom(id, list, options);
+  moduleId = options.base ? moduleId + options.base : moduleId;
+
+  list = list || [];
+
+  if (!stylesInDom[moduleId]) {
+    stylesInDom[moduleId] = [];
+  }
+
+  modulesToDom(moduleId, list, options);
 
   return function update(newList) {
-    addModulesToDom(id, newList || [], options);
+    newList = newList || [];
+
+    if (Object.prototype.toString.call(newList) !== '[object Array]') {
+      return;
+    }
+
+    if (!stylesInDom[moduleId]) {
+      stylesInDom[moduleId] = [];
+    }
+
+    modulesToDom(moduleId, newList, options);
+
+    for (let j = newList.length; j < stylesInDom[moduleId].length; j++) {
+      stylesInDom[moduleId][j]();
+    }
+
+    stylesInDom[moduleId].length = newList.length;
+
+    if (stylesInDom[moduleId].length === 0) {
+      delete stylesInDom[moduleId];
+    }
   };
 };
diff --git a/test/__snapshots__/loader.test.js.snap b/test/__snapshots__/loader.test.js.snap
@@ -362,6 +362,30 @@ exports[`loader should work when the "injectType" option is "lazySingletonStyleT
 
 exports[`loader should work when the "injectType" option is "lazySingletonStyleTag" and ES module syntax used: warnings 1`] = `Array []`;
 
+exports[`loader should work when the "injectType" option is "lazySingletonStyleTag" and files have same name: DOM 1`] = `
+"<!DOCTYPE html><html><head>
+  <title>style-loader test</title>
+  <style id=\\"existing-style\\">.existing { color: yellow }</style>
+<style>.foo {
+  color: red;
+}
+.bar {
+  color: blue;
+}
+</style></head>
+<body>
+  <h1>Body</h1>
+  <div class=\\"target\\"></div>
+  <iframe class=\\"iframeTarget\\"></iframe>
+
+
+</body></html>"
+`;
+
+exports[`loader should work when the "injectType" option is "lazySingletonStyleTag" and files have same name: errors 1`] = `Array []`;
+
+exports[`loader should work when the "injectType" option is "lazySingletonStyleTag" and files have same name: warnings 1`] = `Array []`;
+
 exports[`loader should work when the "injectType" option is "lazySingletonStyleTag": DOM 1`] = `
 "<!DOCTYPE html><html><head>
   <title>style-loader test</title>
@@ -434,6 +458,30 @@ exports[`loader should work when the "injectType" option is "lazyStyleTag" and E
 
 exports[`loader should work when the "injectType" option is "lazyStyleTag" and ES module syntax used: warnings 1`] = `Array []`;
 
+exports[`loader should work when the "injectType" option is "lazyStyleTag" and files have same name: DOM 1`] = `
+"<!DOCTYPE html><html><head>
+  <title>style-loader test</title>
+  <style id=\\"existing-style\\">.existing { color: yellow }</style>
+<style>.foo {
+  color: red;
+}
+</style><style>.bar {
+  color: blue;
+}
+</style></head>
+<body>
+  <h1>Body</h1>
+  <div class=\\"target\\"></div>
+  <iframe class=\\"iframeTarget\\"></iframe>
+
+
+</body></html>"
+`;
+
+exports[`loader should work when the "injectType" option is "lazyStyleTag" and files have same name: errors 1`] = `Array []`;
+
+exports[`loader should work when the "injectType" option is "lazyStyleTag" and files have same name: warnings 1`] = `Array []`;
+
 exports[`loader should work when the "injectType" option is "lazyStyleTag": DOM 1`] = `
 "<!DOCTYPE html><html><head>
   <title>style-loader test</title>
@@ -494,6 +542,24 @@ exports[`loader should work when the "injectType" option is "linkTag" and ES mod
 
 exports[`loader should work when the "injectType" option is "linkTag" and ES module syntax used: warnings 1`] = `Array []`;
 
+exports[`loader should work when the "injectType" option is "linkTag" and files have same name: DOM 1`] = `
+"<!DOCTYPE html><html><head>
+  <title>style-loader test</title>
+  <style id=\\"existing-style\\">.existing { color: yellow }</style>
+<link rel=\\"stylesheet\\" href=\\"04ef9d7491cb6b87d905831b3cb52fdc.css\\"><link rel=\\"stylesheet\\" href=\\"a948740e081d1358c62c536683d63685.css\\"></head>
+<body>
+  <h1>Body</h1>
+  <div class=\\"target\\"></div>
+  <iframe class=\\"iframeTarget\\"></iframe>
+
+
+</body></html>"
+`;
+
+exports[`loader should work when the "injectType" option is "linkTag" and files have same name: errors 1`] = `Array []`;
+
+exports[`loader should work when the "injectType" option is "linkTag" and files have same name: warnings 1`] = `Array []`;
+
 exports[`loader should work when the "injectType" option is "linkTag": DOM 1`] = `
 "<!DOCTYPE html><html><head>
   <title>style-loader test</title>
@@ -560,6 +626,30 @@ exports[`loader should work when the "injectType" option is "singletonStyleTag"
 
 exports[`loader should work when the "injectType" option is "singletonStyleTag" and ES module syntax used: warnings 1`] = `Array []`;
 
+exports[`loader should work when the "injectType" option is "singletonStyleTag" and files have same name: DOM 1`] = `
+"<!DOCTYPE html><html><head>
+  <title>style-loader test</title>
+  <style id=\\"existing-style\\">.existing { color: yellow }</style>
+<style>.foo {
+  color: red;
+}
+.bar {
+  color: blue;
+}
+</style></head>
+<body>
+  <h1>Body</h1>
+  <div class=\\"target\\"></div>
+  <iframe class=\\"iframeTarget\\"></iframe>
+
+
+</body></html>"
+`;
+
+exports[`loader should work when the "injectType" option is "singletonStyleTag" and files have same name: errors 1`] = `Array []`;
+
+exports[`loader should work when the "injectType" option is "singletonStyleTag" and files have same name: warnings 1`] = `Array []`;
+
 exports[`loader should work when the "injectType" option is "singletonStyleTag": DOM 1`] = `
 "<!DOCTYPE html><html><head>
   <title>style-loader test</title>
@@ -632,6 +722,30 @@ exports[`loader should work when the "injectType" option is "styleTag" and ES mo
 
 exports[`loader should work when the "injectType" option is "styleTag" and ES module syntax used: warnings 1`] = `Array []`;
 
+exports[`loader should work when the "injectType" option is "styleTag" and files have same name: DOM 1`] = `
+"<!DOCTYPE html><html><head>
+  <title>style-loader test</title>
+  <style id=\\"existing-style\\">.existing { color: yellow }</style>
+<style>.foo {
+  color: red;
+}
+</style><style>.bar {
+  color: blue;
+}
+</style></head>
+<body>
+  <h1>Body</h1>
+  <div class=\\"target\\"></div>
+  <iframe class=\\"iframeTarget\\"></iframe>
+
+
+</body></html>"
+`;
+
+exports[`loader should work when the "injectType" option is "styleTag" and files have same name: errors 1`] = `Array []`;
+
+exports[`loader should work when the "injectType" option is "styleTag" and files have same name: warnings 1`] = `Array []`;
+
 exports[`loader should work when the "injectType" option is "styleTag": DOM 1`] = `
 "<!DOCTYPE html><html><head>
   <title>style-loader test</title>
diff --git a/test/fixtures/lazy-multiple.js b/test/fixtures/lazy-multiple.js
@@ -0,0 +1,5 @@
+import style from './nested/style.css';
+import otherStyle from './other-nested/style.css';
+
+style.use();
+otherStyle.use();
diff --git a/test/fixtures/multiple.js b/test/fixtures/multiple.js
@@ -0,0 +1,2 @@
+import './nested/style.css';
+import './other-nested/style.css';
diff --git a/test/fixtures/nested/style.css b/test/fixtures/nested/style.css
@@ -0,0 +1,3 @@
+.foo {
+  color: red;
+}
diff --git a/test/fixtures/other-nested/style.css b/test/fixtures/other-nested/style.css
@@ -0,0 +1,3 @@
+.bar {
+  color: blue;
+}
diff --git a/test/loader.test.js b/test/loader.test.js
@@ -287,6 +287,21 @@ describe('loader', () => {
       expect(getErrors(stats)).toMatchSnapshot('errors');
     });
 
+    it(`should work when the "injectType" option is "${injectType}" and files have same name`, async () => {
+      expect.assertions(3);
+
+      const entry = getEntryByInjectType('multiple.js', injectType);
+      const compiler = getCompiler(entry, { injectType });
+      const stats = await compile(compiler);
+
+      runInJsDom('main.bundle.js', compiler, stats, (dom) => {
+        expect(dom.serialize()).toMatchSnapshot('DOM');
+      });
+
+      expect(getWarnings(stats)).toMatchSnapshot('warnings');
+      expect(getErrors(stats)).toMatchSnapshot('errors');
+    });
+
     if (['lazyStyleTag', 'lazySingletonStyleTag'].includes(injectType)) {
       it(`should work when ref is negative when the "injectType" option is "${injectType}"`, async () => {
         expect.assertions(3);
diff --git a/test/manual/index.html b/test/manual/index.html
@@ -70,6 +70,11 @@ <h2>Order</h2>
       <div class="order">BACKGROUND SHOULD BE RED</div>
     </section>
 
+    <section>
+      <h2>Duplicate</h2>
+      <div class="duplicate">SHOULD BE BLUE</div>
+    </section>
+
     <section>
       <h2>Custom element</h2>
       <custom-square l="100" c="red"></custom-square>
diff --git a/test/manual/src/duplicate.css b/test/manual/src/duplicate.css
@@ -0,0 +1,3 @@
+.duplicate {
+  color: red;
+}
diff --git a/test/manual/src/index.js b/test/manual/src/index.js
@@ -9,6 +9,8 @@ import otherStyleLazy from './other-style.lazy.scss';
 import componentLazy from './component.lazy.module.css';
 import './style.link.css';
 import './order.css';
+import './nested.css';
+import './nested/style.css';
 import './custom-square';
 
 console.log('___LOCALS___');
diff --git a/test/manual/src/nested.css b/test/manual/src/nested.css
@@ -0,0 +1,2 @@
+@import './nested/style.css';
+@import './duplicate.css';
diff --git a/test/manual/src/nested/style.css b/test/manual/src/nested/style.css
@@ -0,0 +1,5 @@
+@import '../duplicate.css';
+
+.duplicate {
+  color: blue;
+}
diff --git a/test/runtime/__snapshots__/injectStylesIntoStyleTag.test.js.snap b/test/runtime/__snapshots__/injectStylesIntoStyleTag.test.js.snap
@@ -1,5 +1,9 @@
 // Jest Snapshot v1, https://goo.gl/fbAQLP
 
+exports[`addStyle issue 447 #2 1`] = `"<head><title>Title</title></head><body><h1>Hello world</h1></body>"`;
+
+exports[`addStyle issue 447 #2 2`] = `"<head><title>Title</title></head><body><h1>Hello world</h1></body>"`;
+
 exports[`addStyle issue 447 1`] = `"<head><title>Title</title></head><body><h1>Hello world</h1></body>"`;
 
 exports[`addStyle should throw error with incorrect "insert" option 1`] = `"Couldn't find a style target. This probably means that the value for the 'insert' parameter is invalid."`;
@@ -32,6 +36,24 @@ exports[`addStyle should work with "insert" option 1`] = `"<head><title>Title</t
 
 exports[`addStyle should work with "nonce" attribute and "__webpack_nonce__" variable 1`] = `"<head><title>Title</title><style nonce=\\"87654321\\">.foo { color: red }</style></head><body><h1>Hello world</h1></body>"`;
 
+exports[`addStyle should work with empty modules list #2 1`] = `"<head><title>Title</title></head><body><h1>Hello world</h1></body>"`;
+
+exports[`addStyle should work with empty modules list #2 2`] = `"<head><title>Title</title></head><body><h1>Hello world</h1></body>"`;
+
+exports[`addStyle should work with empty modules list #2 3`] = `"<head><title>Title</title><style>.foo { color: red }</style></head><body><h1>Hello world</h1></body>"`;
+
+exports[`addStyle should work with empty modules list #3 1`] = `"<head><title>Title</title></head><body><h1>Hello world</h1></body>"`;
+
+exports[`addStyle should work with empty modules list #3 2`] = `"<head><title>Title</title></head><body><h1>Hello world</h1></body>"`;
+
+exports[`addStyle should work with empty modules list #3 3`] = `"<head><title>Title</title><style>.foo { color: red }</style></head><body><h1>Hello world</h1></body>"`;
+
+exports[`addStyle should work with empty modules list #3 4`] = `"<head><title>Title</title></head><body><h1>Hello world</h1></body>"`;
+
+exports[`addStyle should work with empty modules list 1`] = `"<head><title>Title</title></head><body><h1>Hello world</h1></body>"`;
+
+exports[`addStyle should work with empty modules list 2`] = `"<head><title>Title</title></head><body><h1>Hello world</h1></body>"`;
+
 exports[`addStyle should work with media 1`] = `"<head><title>Title</title><style media=\\"screen and (min-width:320px)\\">.foo { color: red }</style></head><body><h1>Hello world</h1></body>"`;
 
 exports[`addStyle should work with multiple styles 1`] = `"<head><title>Title</title><style>.foo { color: red }</style><style>.bar { color: blue }</style></head><body><h1>Hello world</h1></body>"`;
@@ -128,3 +150,7 @@ exports[`addStyle should work with updates #14 5`] = `"<head><title>Title</title
 exports[`addStyle should work with updates 1`] = `"<head><title>Title</title><style>.foo { color: red }</style></head><body><h1>Hello world</h1></body>"`;
 
 exports[`addStyle should work with updates 2`] = `"<head><title>Title</title><style>.foo { color: blue }</style></head><body><h1>Hello world</h1></body>"`;
+
+exports[`addStyle should work without modules 1`] = `"<head><title>Title</title></head><body><h1>Hello world</h1></body>"`;
+
+exports[`addStyle should work without modules 2`] = `"<head><title>Title</title></head><body><h1>Hello world</h1></body>"`;
diff --git a/test/runtime/injectStylesIntoStyleTag.test.js b/test/runtime/injectStylesIntoStyleTag.test.js

Original file line number	Diff line number	Diff line change
`@@ -0,0 +1,2 @@`
	`1`	`+import './nested/style.css';`
	`2`	`+import './other-nested/style.css';`
Original file line number	Diff line number	Diff line change
`@@ -0,0 +1,3 @@`
	`1`	`+.duplicate {`
	`2`	`+ color: red;`
	`3`	`+}`