Repository: webpack-contrib/css-loader
Pull request #1193 (2020-09-22 20:38:20)

feat: add fallback if custom getLocalIdent returns null
This PR contains a:

- [ ] **bugfix**
- [X] new **feature**
- [ ] **code refactor**
- [X] **test update**
- [ ] **typo fix**
- [ ] **metadata update**

### Motivation / Use-Case

With the release of the 4.x version of the css-loader, the behavior that allowed a `getLocalIdent` function to return a falsy value to fallback to the default `getLocalIdent` function was lost. That original behavior was introduced in [this pull request](https://github.com/webpack-contrib/css-loader/pull/963).

This PR reintroduces that behavior, but instead requires the function to return `null` or `undefined`.

### Breaking Changes

n/a

### Additional Info


jsf-clabot: [![CLA assistant check](https://cla.js.foundation/pull/badge/not_signed)](https://cla.js.foundation/webpack-contrib/css-loader?pullRequest=1193) <br/>Thank you for your submission, we really appreciate it. Like many open source projects, we ask that you sign our [Contributor License Agreement](https://cla.js.foundation/webpack-contrib/css-loader?pullRequest=1193) before we can accept your contribution.<br/>
codecov[bot]: # [Codecov](https://codecov.io/gh/webpack-contrib/css-loader/pull/1193?src=pr&el=h1) Report
> Merging [#1193](https://codecov.io/gh/webpack-contrib/css-loader/pull/1193?src=pr&el=desc) into [master](https://codecov.io/gh/webpack-contrib/css-loader/commit/df111b88b062d7062adcdafde44e93d2ed1c77ec?el=desc) will **decrease** coverage by `0.14%`.
> The diff coverage is `100.00%`.

[![Impacted file tree graph](https://codecov.io/gh/webpack-contrib/css-loader/pull/1193/graphs/tree.svg?width=650&height=150&src=pr&token=DlN3OQ0JnO)](https://codecov.io/gh/webpack-contrib/css-loader/pull/1193?src=pr&el=tree)

```diff
@@            Coverage Diff             @@
##           master    #1193      +/-   ##
==========================================
- Coverage   99.24%   99.10%   -0.15%     
==========================================
  Files          10       10              
  Lines         666      670       +4     
  Branches      212      213       +1     
==========================================
+ Hits          661      664       +3     
- Misses          5        6       +1     
```


| [Impacted Files](https://codecov.io/gh/webpack-contrib/css-loader/pull/1193?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [src/utils.js](https://codecov.io/gh/webpack-contrib/css-loader/pull/1193/diff?src=pr&el=tree#diff-c3JjL3V0aWxzLmpz) | `98.03% <100.00%> (-0.31%)` | :arrow_down: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/webpack-contrib/css-loader/pull/1193?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/webpack-contrib/css-loader/pull/1193?src=pr&el=footer). Last update [df111b8...96ba438](https://codecov.io/gh/webpack-contrib/css-loader/pull/1193?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).

tvsbrent: Sorry, I had to fix a commit subject to correct the lint error.

Patch:
diff --git a/README.md b/README.md
index ce5984a..9eaae51 100644
--- a/README.md
+++ b/README.md
@@ -848,6 +848,8 @@ Default: `undefined`
 
 Allows to specify a function to generate the classname.
 By default we use built-in function to generate a classname.
+If the custom function returns `null` or `undefined`, we fallback to the
+built-in function to generate the classname.
 
 **webpack.config.js**
 
@@ -1245,7 +1247,7 @@ module.exports = {
 
 ### Separating `Interoperable CSS`-only and `CSS Module` features
 
-The following setup is an example of allowing `Interoperable CSS` features only (such as `:import` and `:export`) without using further `CSS Module` functionality by setting `compileType` option for all files that do not match `*.module.scss` naming convention. This is for reference as having `ICSS` features applied to all files was default `css-loader` behavior before v4.  
+The following setup is an example of allowing `Interoperable CSS` features only (such as `:import` and `:export`) without using further `CSS Module` functionality by setting `compileType` option for all files that do not match `*.module.scss` naming convention. This is for reference as having `ICSS` features applied to all files was default `css-loader` behavior before v4.
 Meanwhile all files matching `*.module.scss` are treated as `CSS Modules` in this example.
 
 An example case is assumed where a project requires canvas drawing variables to be synchronized with CSS - canvas drawing uses the same color (set by color name in JavaScript) as HTML background (set by class name in CSS).
diff --git a/src/utils.js b/src/utils.js
index ca167e2..218c0b7 100644
--- a/src/utils.js
+++ b/src/utils.js
@@ -281,11 +281,32 @@ function getModulesPlugins(options, loaderContext) {
       extractImports(),
       modulesScope({
         generateScopedName(exportName) {
-          return getLocalIdent(loaderContext, localIdentName, exportName, {
-            context: localIdentContext,
-            hashPrefix: localIdentHashPrefix,
-            regExp: localIdentRegExp,
-          });
+          let localIdent = getLocalIdent(
+            loaderContext,
+            localIdentName,
+            exportName,
+            {
+              context: localIdentContext,
+              hashPrefix: localIdentHashPrefix,
+              regExp: localIdentRegExp,
+            }
+          );
+
+          // A null/undefined value signals that we should invoke the default
+          // getLocalIdent method.
+          if (localIdent == null) {
+            localIdent = defaultGetLocalIdent(
+              loaderContext,
+              localIdentName,
+              exportName,
+              {
+                context: localIdentContext,
+                hashPrefix: localIdentHashPrefix,
+                regExp: localIdentRegExp,
+              }
+            );
+          }
+          return 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 b57716f..da337d5 100644
--- a/test/__snapshots__/modules-option.test.js.snap
+++ b/test/__snapshots__/modules-option.test.js.snap
@@ -487,6 +487,49 @@ exports[`"modules" option issue #1063: result 1`] = `
 
 exports[`"modules" option issue #1063: warnings 1`] = `Array []`;
 
+exports[`"modules" option issue #1191 - fallback to default getLocalIdent: errors 1`] = `Array []`;
+
+exports[`"modules" option issue #1191 - fallback to default getLocalIdent: 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, \\".some-class {\\\\n  color: red;\\\\n}\\\\n\\", \\"\\"]);
+// Exports
+___CSS_LOADER_EXPORT___.locals = {
+	\\"some-class\\": \\"some-class\\"
+};
+export default ___CSS_LOADER_EXPORT___;
+"
+`;
+
+exports[`"modules" option issue #1191 - fallback to default getLocalIdent: result 1`] = `
+Object {
+  "css1": Array [
+    Array [
+      "./modules/issue-1191/issue-1191.css",
+      ".some-class {
+  color: red;
+}
+",
+      "",
+    ],
+  ],
+  "css2": Array [
+    Array [
+      "./modules/issue-1191/issue-1191-custom.css",
+      ".custom-some-class {
+  color: red;
+}
+",
+      "",
+    ],
+  ],
+}
+`;
+
+exports[`"modules" option issue #1191 - fallback to default getLocalIdent: warnings 1`] = `Array []`;
+
 exports[`"modules" option should avoid unnecessary "require": errors 1`] = `Array []`;
 
 exports[`"modules" option should avoid unnecessary "require": module 1`] = `
diff --git a/test/fixtures/modules/issue-1191/issue-1191-custom.css b/test/fixtures/modules/issue-1191/issue-1191-custom.css
new file mode 100644
index 0000000..b58cc07
--- /dev/null
+++ b/test/fixtures/modules/issue-1191/issue-1191-custom.css
@@ -0,0 +1,3 @@
+.some-class {
+  color: red;
+}
diff --git a/test/fixtures/modules/issue-1191/issue-1191.css b/test/fixtures/modules/issue-1191/issue-1191.css
new file mode 100644
index 0000000..b58cc07
--- /dev/null
+++ b/test/fixtures/modules/issue-1191/issue-1191.css
@@ -0,0 +1,3 @@
+.some-class {
+  color: red;
+}
diff --git a/test/fixtures/modules/issue-1191/issue-1191.js b/test/fixtures/modules/issue-1191/issue-1191.js
new file mode 100644
index 0000000..940dbfb
--- /dev/null
+++ b/test/fixtures/modules/issue-1191/issue-1191.js
@@ -0,0 +1,8 @@
+import css1 from './issue-1191.css';
+import css2 from './issue-1191-custom.css';
+
+const wrapper = { css1, css2 }
+
+__export__ = wrapper;
+
+export default wrapper;
diff --git a/test/modules-option.test.js b/test/modules-option.test.js
index a31c2f1..0a7f4b0 100644
--- a/test/modules-option.test.js
+++ b/test/modules-option.test.js
@@ -657,6 +657,28 @@ describe('"modules" option', () => {
     expect(getErrors(stats)).toMatchSnapshot('errors');
   });
 
+  it('issue #1191 - fallback to default getLocalIdent', async () => {
+    const compiler = getCompiler('./modules/issue-1191/issue-1191.js', {
+      modules: {
+        getLocalIdent: (ctx, localIdentName, localName) =>
+          ctx.resourcePath.includes('custom') ? `custom-${localName}` : null,
+        localIdentName: '[local]',
+      },
+    });
+    const stats = await compile(compiler);
+
+    expect(
+      getModuleSource('./modules/issue-1191/issue-1191.css', stats)
+    ).toMatchSnapshot('module');
+
+    expect(getExecutedCode('main.bundle.js', compiler, stats)).toMatchSnapshot(
+      'result'
+    );
+
+    expect(getWarnings(stats)).toMatchSnapshot('warnings');
+    expect(getErrors(stats)).toMatchSnapshot('errors');
+  });
+
   it('should work with the `exportGlobals` option (the `mode` option is `global`)', async () => {
     const compiler = getCompiler(
       './modules/exportGlobals-global/exportGlobals.js',