Repository: webpack-contrib/css-loader
Pull request #965 (2019-07-08 15:24:18)

fix: default context should be undefined instead of null
`loaderUtils.interpolateName` breaks if it passes `null` to path.relative

This PR contains a:

- [x] **bugfix**
- [ ] new **feature**
- [ ] **code refactor**
- [ ] **test update** <!-- if bug or feature is checked, this should be too -->
- [ ] **typo fix**
- [ ] **metadata update**

### Motivation / Use-Case

When css-loader 3.0.0 is used with `react-dev-utils/getCSSModuleLocalIdent` the build fails because getLocalIdent passes [`context` to `loaderUtils.interpolateName`](https://github.com/facebook/create-react-app/blob/master/packages/react-dev-utils/getCSSModuleLocalIdent.js#L34). interpolateName then uses this `context` to [get a relative path using `path.relative()`](https://github.com/webpack/loader-utils/blob/master/lib/interpolateName.js#L71). While `undefined` (the value in ^2.1.1) is ... undefined and thus simply ignored by `path.relative()`, `null` is an object and causes the error below (shortened):

```
TypeError [ERR_INVALID_ARG_TYPE]: The "from" argument must be of type string. Received type object
    at C:\htdocs\test\css-loader-issue\test.module.css:1:1
    at validateString (internal/validators.js:107:11)
    at Object.relative (path.js:434:5)
    at Object.interpolateName (C:\htdocs\test\css-loader-issue\node_modules\loader-utils\lib\interpolateName.js:71:10)
    at Object.getLocalIdent (C:\htdocs\test\css-loader-issue\webpack.config.js:22:37)
    at generateScopedName (C:\htdocs\_git\css-loader\dist\utils.js:142:39)
    [...]
```

This PR removes the `context` default option completely, implicitly setting its value back to `undefined`.

### Breaking Changes

Existing tests are still passing so I assume there should be no breaking changes. I didn't add any additional tests though.

### Additional Info

You should be able to reproduce this behavior by cloning my [demo repo](https://github.com/manuelbieh/css-loader-issue) and run `npm run build`.




evilebottnawi: strange, we use `options.context || ''` so it should be empty lint and no problems
manuelbieh: Yes, you do, but only in your internal getLocalIdent() function in utils.js. If a custom getLocalIdent function is passed as option:
```js
modules: { 
  getLocalIdent: (loaderContext, localIdentName, localName, options) => {
    // this should be undefined:
    options.context === null
  }
}
```

… it's `null` instead of `undefined`. So if third-party getLocalIdent functions want to pass context to `path.relative()` they must take care of checking it for `null || ''` themselves.
manuelbieh: … you can check it by adding a `console.log(options)` to `node_modules/loader-utils/lib/interpolateName.js:39`. It is:
```
{ context: null, hashPrefix: '', regExp: null }
```
in 3.0.0 and
```
{ regExp: undefined, hashPrefix: '', context: undefined }
```
in 2.1.1.

`context` being `null` instead of `undefined` causes the `interpolateName` function to fail because of this line (70):
```
      directory = path
        .relative(context, resourcePath + '_')
```
evilebottnawi: hm, looks regression, let's add test for this case
manuelbieh: Just added a test. Please have a look!

Patch:
diff --git a/src/utils.js b/src/utils.js
index 4b9f0b9..e23bb8d 100644
--- a/src/utils.js
+++ b/src/utils.js
@@ -120,7 +120,6 @@ function getModulesPlugins(options, loaderContext) {
     mode: 'local',
     localIdentName: '[hash:base64]',
     getLocalIdent,
-    context: null,
     hashPrefix: '',
     localIdentRegExp: null,
   };


Test patch:
diff --git a/test/__snapshots__/modules-option.test.js.snap b/test/__snapshots__/modules-option.test.js.snap
index b906387..d3e52c6 100644
--- a/test/__snapshots__/modules-option.test.js.snap
+++ b/test/__snapshots__/modules-option.test.js.snap
@@ -6564,6 +6564,36 @@ Array [
 
 exports[`modules should correctly replace escaped symbols in selector with localIdentName option: warnings 1`] = `Array []`;
 
+exports[`modules should have an undefined context if no context was given: errors 1`] = `Array []`;
+
+exports[`modules should have an undefined context if no context was given: locals 1`] = `
+Object {
+  "abc": "foo",
+  "def": "foo",
+  "ghi": "foo",
+  "jkl": "foo",
+}
+`;
+
+exports[`modules should have an undefined context if no context was given: module (evaluated) 1`] = `
+Array [
+  Array [
+    1,
+    ".foo .foo {
+  color: red;
+}
+
+.foo .foo {
+  color: blue;
+}
+",
+    "",
+  ],
+]
+`;
+
+exports[`modules should have an undefined context if no context was given: warnings 1`] = `Array []`;
+
 exports[`modules should prefixes leading hyphen + digit with underscore with localIdentName option: errors 1`] = `Array []`;
 
 exports[`modules should prefixes leading hyphen + digit with underscore with localIdentName option: locals 1`] = `
diff --git a/test/modules-option.test.js b/test/modules-option.test.js
index 73630a7..a412de0 100644
--- a/test/modules-option.test.js
+++ b/test/modules-option.test.js
@@ -288,6 +288,31 @@ describe('modules', () => {
     expect(stats.compilation.errors).toMatchSnapshot('errors');
   });
 
+  it('should have an undefined context if no context was given', async () => {
+    const config = {
+      loader: {
+        options: {
+          modules: {
+            getLocalIdent(loaderContext, localIdentName, localName, options) {
+              expect(options.context).toBeUndefined();
+              return 'foo';
+            },
+          },
+        },
+      },
+    };
+    const testId = './modules/getLocalIdent.css';
+    const stats = await webpack(testId, config);
+    const { modules } = stats.toJson();
+    const module = modules.find((m) => m.id === testId);
+    const evaluatedModule = evaluated(module.source);
+
+    expect(evaluatedModule).toMatchSnapshot('module (evaluated)');
+    expect(evaluatedModule.locals).toMatchSnapshot('locals');
+    expect(stats.compilation.warnings).toMatchSnapshot('warnings');
+    expect(stats.compilation.errors).toMatchSnapshot('errors');
+  });
+
   it('should respects getLocalIdent option (global mode)', async () => {
     const config = {
       loader: {