Repository: webpack-contrib/css-loader (js)

Issue:
values in selectors aren't escaped properly.
on latest css-loader. I'd expect this to be true for any defined value but not importing classes where the local ident is generated.

// button.css
```css
.button {

}
```
// toolbar.css
```css
@value btn from './button.css'

.toolbar > btn {
	color: red
}
```

```js
rules: [
  { 
  test: /\.css/, 
  use: {
    loader: 'css-loader',
    options: {
		modules: {
			getLocalIdent: (ctx, name) => `${name}.hey`
		}
	}
  }
]
```

The local names would need escaping and webpack outputs:

```
.button\.hey {
}
```

which is correctly escaped but in toolbar its

```css
.toolbar\.hey > .button.hey {
}
```

note that the interpolated value isn't escaped. I'm not sure why this is happening maybe it needs to be re-escaped?

Pull request:
Fix escape getLocalIdent
<!--
  HOLY CRAP a Pull Request. We ❤️ those!

  If you remove or skip this template, you'll make the 🐼 sad and the mighty god
  of Github will appear and pile-drive the close button from a great height
  while making animal noises.

  Please place an x (no spaces!) in all [ ] that apply
-->

This PR contains a:

- [x] **bugfix**
- [ ] new **feature**
- [ ] **code refactor**
- [x] **test update** <!-- if bug or feature is checked, this should be too -->
- [ ] **typo fix**
- [ ] **metadata update**

### Motivation / Use-Case

Fixes #966 

### Breaking Changes

Yes. `.` is replaced in classname

### Additional Info

No



Patch:
diff --git a/src/utils.js b/src/utils.js
index 218c0b7..dd40967 100644
--- 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,
       }),


Test patch:
diff --git a/test/__snapshots__/modules-option.test.js.snap b/test/__snapshots__/modules-option.test.js.snap
index da337d5..c50f132 100644
--- a/test/__snapshots__/modules-option.test.js.snap
+++ b/test/__snapshots__/modules-option.test.js.snap
@@ -137,6 +137,58 @@ 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`] = `
@@ -144,10 +196,10 @@ exports[`"modules" option issue #966: module 1`] = `
 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
new file mode 100644
index 0000000..a27ca56
--- /dev/null
+++ 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
new file mode 100644
index 0000000..99ef679
--- /dev/null
+++ 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
index 0a7f4b0..06b7cd2 100644
--- 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: {