Repository: styled-components/babel-plugin-styled-components
Pull request #304 (2020-09-23 09:35:38)

Add "meaninglessFileNames" option
This PR adds a new option: `meaninglessFileNames`.

Consider the following use case:

- A developer enables `babel-plugin-styled-components` with the defaults of `displayName: true` and `fileName: true`.
- They organize their components with the following structure:

    ```
    src/
      components/
        MyComponent/
          index.js
          styles.js
    ```

    `styles.js` would be the file where they place their styled components.
- For consistency, they always use `Container` for the root element of every component:

    ```jsx
    // index.js
    import React from 'react';
    import { Container } from './styles.js';

    const MyComponent = () => <Container>Hello world!</Container>;
    ```

The class name generated for the `Container` element of `MyComponent` would be something like `styles__Container-sc-000000-0`. But it would be the same for the rest of the components of the application, because their styled components would be defined in `styles.js` too. So there would be no way to differentiate two `Container`s from different components.

The easy soultion would be to define the styled components in `index.js`, but for large components that can get messy very quick. Another solution would be to name each to prefix each styled component with the component name (i.e: `MyComponentContainer`), but that is very redundant and can make the code harder to read.

Actually, that is the exact use case of our company, and we found that many of our front-end developers ended up using the prefix solution. They preferred to compromise a bit on the readability of the code to allow for a better debugging experience.

This PR introduces the ideal solution: it allows to control when should the directory name be used for the component display name, and when it shouldn't, through the `meaninglessFileNames` option. By using the following options:

```js
{
  displayName: true,
  fileName: true,
  meaninglessFileNames: ['index', 'styles']
},
```

our previous example would be labeled as `MyComponent__Container-sc-000000-0`, which is exactly what we need.

If this option is omitted, it defaults to `['index']`, leaving the previous behaviour intact.

If this option is specified but either `displayName` or `fileName` are set to false, then it has no effect.

### Some points to consider

1. I didn't test this because I'm unsure on how to make this work with the test suite. It appears that the source file must be named `code.js`. How would I test that it works with different values? Any guidance would be appreciated.
2. I'm unsure on whether or not `meaninglessFileNames` is the best naming. I considered `ignoredFileNames`, but that might be confusing as it might suggest that the files defined there would be ignored by the plugin. I also considered `overruledFileNames`, but I thought that `meaninglessFileNames` is what describes best what this option is about. Any thoughts?

Thanks for your time!

MeLlamoPablo: Hello! Could I please get some feedback on this PR?

I understand if this feature doesn't align with the project's design goals, but I'd appreciate if anyone let me know whether this has any chance of getting merged. Of course, I'm willing to put more work into this.
MeLlamoPablo: @probablyup rebased this branch on top of main and added some tests! Let me know if they look good :)

By the way, while digging deeper in the testing system I found that the case where the directory name isn't used when the file is named "index" is not being tested. I assume this is because [babel-test doesn't allow us to change the `code.js` file name as they have it hardcoded](https://github.com/satya164/babel-test/blob/9ff771fd11c258884bbe5564945e66d9b269a19a/src/index.js#L81).

Maybe it's worth sending them a PR to allow to change `code.js` to `index.js` for this specific case? Probably the author didn't think of an edge case where the filename would be relevant to the test itself.
probablyup: > Maybe it's worth sending them a PR to allow to change code.js to index.js for this specific case? Probably the author didn't think of an edge case where the filename would be relevant to the test itself.

Love that idea!

Patch:
diff --git a/src/utils/options.js b/src/utils/options.js
index e653545..509fd9d 100644
--- a/src/utils/options.js
+++ b/src/utils/options.js
@@ -9,6 +9,7 @@ export const useTopLevelImportPathMatchers = state =>
   getOption(state, 'topLevelImportPaths', []).map(pattern => new RegExp(pattern))
 export const useSSR = state => getOption(state, 'ssr', true)
 export const useFileName = state => getOption(state, 'fileName')
+export const useMeaninglessFileNames = state => getOption(state, 'meaninglessFileNames', ['index'])
 export const useMinify = state => getOption(state, 'minify')
 export const useTranspileTemplateLiterals = state =>
   getOption(state, 'transpileTemplateLiterals')
diff --git a/src/visitors/displayNameAndId.js b/src/visitors/displayNameAndId.js
index af20ae4..7663ce3 100644
--- a/src/visitors/displayNameAndId.js
+++ b/src/visitors/displayNameAndId.js
@@ -4,6 +4,7 @@ import {
   useFileName,
   useDisplayName,
   useSSR,
+  useMeaninglessFileNames,
   useNamespace,
 } from '../utils/options'
 import getName from '../utils/getName'
@@ -111,21 +112,22 @@ const getExistingConfig = t => path => {
   }
 }
 
-const getBlockName = file => {
+const getBlockName = (file, meaninglessFileNames) => {
   const name = path.basename(
     file.opts.filename,
     path.extname(file.opts.filename)
   )
-  return name !== 'index'
-    ? name
-    : path.basename(path.dirname(file.opts.filename))
+
+  return meaninglessFileNames.includes(name)
+    ? path.basename(path.dirname(file.opts.filename))
+    : name
 }
 
 const getDisplayName = t => (path, state) => {
   const { file } = state
   const componentName = getName(t)(path)
   if (file) {
-    const blockName = getBlockName(file)
+    const blockName = getBlockName(file, useMeaninglessFileNames(state))
     if (blockName === componentName) {
       return componentName
     }


Test patch:
diff --git a/test/fixtures/use-directory-name/.babelrc b/test/fixtures/use-directory-name/.babelrc
new file mode 100644
index 0000000..3921aea
--- /dev/null
+++ b/test/fixtures/use-directory-name/.babelrc
@@ -0,0 +1,12 @@
+{
+  "plugins": [
+    [
+      "../../../src",
+      {
+        "meaninglessFileNames": ["code"],
+        "transpileTemplateLiterals": false,
+        "ssr": true
+      }
+    ]
+  ]
+}
diff --git a/test/fixtures/use-directory-name/code.js b/test/fixtures/use-directory-name/code.js
new file mode 100644
index 0000000..e656c5d
--- /dev/null
+++ b/test/fixtures/use-directory-name/code.js
@@ -0,0 +1,6 @@
+import styled from "styled-components";
+
+const Test = styled.div`color: red;`;
+const before = styled.div`color: blue;`;
+styled.div``;
+export default styled.button``;
diff --git a/test/fixtures/use-directory-name/output.js b/test/fixtures/use-directory-name/output.js
new file mode 100644
index 0000000..f1b7488
--- /dev/null
+++ b/test/fixtures/use-directory-name/output.js
@@ -0,0 +1,17 @@
+import styled from "styled-components";
+const Test = styled.div.withConfig({
+  displayName: "use-directory-name__Test",
+  componentId: "sc-193y009-0"
+})`color:red;`;
+const before = styled.div.withConfig({
+  displayName: "use-directory-name__before",
+  componentId: "sc-193y009-1"
+})`color:blue;`;
+styled.div.withConfig({
+  displayName: "use-directory-name",
+  componentId: "sc-193y009-2"
+})``;
+export default styled.button.withConfig({
+  displayName: "use-directory-name",
+  componentId: "sc-193y009-3"
+})``;