feat: escape getLocalIdent by default (#1196)

BREAKING CHANGE: returned value from the `getLocalIdent` escapes by default, the `exportName` value is always unescaped
webpack-contrib · Sep 24, 2020 · d779eb1 · d779eb1
1 parent dd52931
commit d779eb1
Show file tree

Hide file tree

Showing 5 changed files with 157 additions and 29 deletions.
diff --git a/src/utils.js b/src/utils.js
@@ -49,6 +49,18 @@ const filenameReservedRegex = /[<>:"/\\|?*]/g;
 // eslint-disable-next-line no-control-regex
 const reControlChars = /[\u0000-\u001f\u0080-\u009f]/g;
 
+function escapeLocalident(localident) {
+  return cssesc(
+    localident
+      // For `[hash]` placeholder
+      .replace(/^((-?[0-9])|--)/, '_$1')
+      .replace(filenameReservedRegex, '-')
+      .replace(reControlChars, '-')
+      .replace(/\./g, '-'),
+    { isIdentifier: true }
+  );
+}
+
 function defaultGetLocalIdent(
   loaderContext,
   localIdentName,
@@ -60,19 +72,9 @@ function defaultGetLocalIdent(
   const request = normalizePath(path.relative(context, resourcePath));
 
   // eslint-disable-next-line no-param-reassign
-  options.content = `${hashPrefix + request}\x00${unescape(localName)}`;
+  options.content = `${hashPrefix + request}\x00${localName}`;
 
-  // Using `[path]` placeholder outputs `/` we need escape their
-  // Also directories can contains invalid characters for css we need escape their too
-  return cssesc(
-    interpolateName(loaderContext, localIdentName, options)
-      // For `[hash]` placeholder
-      .replace(/^((-?[0-9])|--)/, '_$1')
-      .replace(filenameReservedRegex, '-')
-      .replace(reControlChars, '-')
-      .replace(/\./g, '-'),
-    { isIdentifier: true }
-  ).replace(/\\\[local\\]/gi, localName);
+  return interpolateName(loaderContext, localIdentName, options);
 }
 
 function normalizeUrl(url, isStringValue) {
@@ -143,7 +145,8 @@ function getModulesOptions(rawOptions, loaderContext) {
     localIdentHashPrefix: '',
     // eslint-disable-next-line no-undefined
     localIdentRegExp: undefined,
-    getLocalIdent: defaultGetLocalIdent,
+    // eslint-disable-next-line no-undefined
+    getLocalIdent: undefined,
     namedExport: false,
     exportLocalsConvention: 'asIs',
     exportOnlyLocals: false,
@@ -281,32 +284,42 @@ function getModulesPlugins(options, loaderContext) {
       extractImports(),
       modulesScope({
         generateScopedName(exportName) {
-          let localIdent = getLocalIdent(
-            loaderContext,
-            localIdentName,
-            exportName,
-            {
-              context: localIdentContext,
-              hashPrefix: localIdentHashPrefix,
-              regExp: localIdentRegExp,
-            }
-          );
+          let localIdent;
+
+          if (typeof getLocalIdent !== 'undefined') {
+            localIdent = getLocalIdent(
+              loaderContext,
+              localIdentName,
+              unescape(exportName),
+              {
+                context: localIdentContext,
+                hashPrefix: localIdentHashPrefix,
+                regExp: localIdentRegExp,
+              }
+            );
+          }
 
           // A null/undefined value signals that we should invoke the default
           // getLocalIdent method.
-          if (localIdent == null) {
+          if (typeof localIdent === 'undefined' || localIdent === null) {
             localIdent = defaultGetLocalIdent(
               loaderContext,
               localIdentName,
-              exportName,
+              unescape(exportName),
               {
                 context: localIdentContext,
                 hashPrefix: localIdentHashPrefix,
                 regExp: localIdentRegExp,
               }
             );
+
+            return escapeLocalident(localIdent).replace(
+              /\\\[local\\]/gi,
+              exportName
+            );
           }
-          return localIdent;
+
+          return escapeLocalident(localIdent);
         },
         exportGlobals: options.modules.exportGlobals,
       }),

diff --git a/test/__snapshots__/modules-option.test.js.snap b/test/__snapshots__/modules-option.test.js.snap
@@ -137,17 +137,69 @@ Array [
 
 exports[`"modules" option issue #861: warnings 1`] = `Array []`;
 
+exports[`"modules" option issue #966 - values in selectors aren't escaped properly: errors 1`] = `Array []`;
+
+exports[`"modules" option issue #966 - values in selectors aren't escaped properly: module 1`] = `
+"// Imports
+import ___CSS_LOADER_API_IMPORT___ from \\"../../../../src/runtime/api.js\\";
+var ___CSS_LOADER_EXPORT___ = ___CSS_LOADER_API_IMPORT___(false);
+// Module
+___CSS_LOADER_EXPORT___.push([module.id, \\"._7-foo-class {\\\\n  color: red;\\\\n}\\\\n\\\\n.\\\\\\\\--bar-class {\\\\n  color: red;\\\\n}\\\\n\\\\n.\\\\\\\\--baz-class {\\\\n  color: red;\\\\n}\\\\n\\\\n.fooBaz-class-continuation {\\\\n  color: red;\\\\n}\\\\n\\\\n.some.class {\\\\n  color: red;\\\\n}\\\\n\\", \\"\\"]);
+// Exports
+___CSS_LOADER_EXPORT___.locals = {
+	\\"foo-class\\": \\"_7-foo-class\\",
+	\\"bar-class\\": \\"--bar-class\\",
+	\\"baz-class\\": \\"--baz-class\\",
+	\\"fooBaz-class\\": \\"fooBaz-class-continuation\\",
+	\\"some\\": \\"some\\",
+	\\"class\\": \\"class\\"
+};
+export default ___CSS_LOADER_EXPORT___;
+"
+`;
+
+exports[`"modules" option issue #966 - values in selectors aren't escaped properly: result 1`] = `
+Array [
+  Array [
+    "./modules/issue-966/issue-966.css",
+    "._7-foo-class {
+  color: red;
+}
+
+.\\\\--bar-class {
+  color: red;
+}
+
+.\\\\--baz-class {
+  color: red;
+}
+
+.fooBaz-class-continuation {
+  color: red;
+}
+
+.some.class {
+  color: red;
+}
+",
+    "",
+  ],
+]
+`;
+
+exports[`"modules" option issue #966 - values in selectors aren't escaped properly: warnings 1`] = `Array []`;
+
 exports[`"modules" option issue #966: errors 1`] = `Array []`;
 
 exports[`"modules" option issue #966: module 1`] = `
 "// Imports
 import ___CSS_LOADER_API_IMPORT___ from \\"../../../../src/runtime/api.js\\";
 var ___CSS_LOADER_EXPORT___ = ___CSS_LOADER_API_IMPORT___(false);
 // Module
-___CSS_LOADER_EXPORT___.push([module.id, \\".button.hey {\\\\n  color: red;\\\\n}\\\\n\\", \\"\\"]);
+___CSS_LOADER_EXPORT___.push([module.id, \\".button-hey {\\\\n  color: red;\\\\n}\\\\n\\", \\"\\"]);
 // Exports
 ___CSS_LOADER_EXPORT___.locals = {
-	\\"button\\": \\"button.hey\\"
+	\\"button\\": \\"button-hey\\"
 };
 export default ___CSS_LOADER_EXPORT___;
 "
@@ -157,7 +209,7 @@ exports[`"modules" option issue #966: result 1`] = `
 Array [
   Array [
     "./modules/issue-966/button.css",
-    ".button.hey {
+    ".button-hey {
   color: red;
 }
 ",

diff --git a/test/fixtures/modules/issue-966/issue-966.css b/test/fixtures/modules/issue-966/issue-966.css
@@ -0,0 +1,19 @@
+.foo-class {
+  color: red;
+}
+
+.bar-class {
+  color: red;
+}
+
+.baz-class {
+  color: red;
+}
+
+.fooBaz-class {
+  color: red;
+}
+
+.some.class {
+  color: red;
+}
diff --git a/test/fixtures/modules/issue-966/issue-966.js b/test/fixtures/modules/issue-966/issue-966.js
@@ -0,0 +1,5 @@
+import css from './issue-966.css';
+
+__export__ = css;
+
+export default css;
diff --git a/test/modules-option.test.js b/test/modules-option.test.js
@@ -491,6 +491,45 @@ describe('"modules" option', () => {
     expect(getErrors(stats)).toMatchSnapshot('errors');
   });
 
+  it("issue #966 - values in selectors aren't escaped properly", async () => {
+    const compiler = getCompiler('./modules/issue-966/issue-966.js', {
+      modules: {
+        getLocalIdent: (loaderContext, localIdentName, localName) => {
+          if (localName === 'foo-class') {
+            return `7-${localName}`;
+          }
+
+          if (localName === 'bar-class') {
+            return `>-${localName}`;
+          }
+
+          if (localName === 'baz-class') {
+            return `\u0000-${localName}`;
+          }
+
+          if (localName === 'fooBaz-class') {
+            return `${localName}.continuation`;
+          }
+
+          return null;
+        },
+        localIdentName: '[local]',
+      },
+    });
+    const stats = await compile(compiler);
+
+    expect(
+      getModuleSource('./modules/issue-966/issue-966.css', stats)
+    ).toMatchSnapshot('module');
+
+    expect(getExecutedCode('main.bundle.js', compiler, stats)).toMatchSnapshot(
+      'result'
+    );
+
+    expect(getWarnings(stats)).toMatchSnapshot('warnings');
+    expect(getErrors(stats)).toMatchSnapshot('errors');
+  });
+
   it('issue #967', async () => {
     const compiler = getCompiler('./modules/issue-967/path-placeholder.js', {
       modules: {