Repository: styled-components/babel-plugin-styled-components
Pull request #313 (2021-02-11 13:36:24)

Ensure sc- prefix is always added
Currently, the sc- is added at the start of the hash by replacing the first digit. The problem is that the hash may also start by a letter, causing the class name to sometimes miss the sc- prefix.

This fix makes it possible to use this plugin with styled-components@5 + jest-styled-components@7.

In the meantime, I've published the fix on npm under [@chalbert/babel-plugin-styled-components](https://www.npmjs.com/package/@chalbert/babel-plugin-styled-components)

In my experience, it fixes the following issues :

#268
styled-components/jest-styled-components#297
styled-components/jest-styled-components#335
styled-components/jest-styled-components#294
And potentially others.

agriffis: What about something like this instead?
- generated classes get consistently namespaced or `sc-` prefix
- block names get an underscore prefix when needed

```
diff --git a/src/utils/prefixDigit.js b/src/utils/prefixDigit.js
index 8cb8c4b..ec5d2ee 100644
--- a/src/utils/prefixDigit.js
+++ b/src/utils/prefixDigit.js
@@ -1,3 +1,3 @@
-export default function prefixLeadingDigit(str) {
-  return str.replace(/^(\d)/, 'sc-$1')
+export default function prefixLeadingDigit(str, prefix) {
+  return str.replace(/^(?=\d)/, prefix)
 }
diff --git a/src/visitors/displayNameAndId.js b/src/visitors/displayNameAndId.js
index 42c1c1f..eed1140 100644
--- a/src/visitors/displayNameAndId.js
+++ b/src/visitors/displayNameAndId.js
@@ -72,8 +72,8 @@ const getDisplayName = t => (path, state) => {
       return componentName
     }
     return componentName
-      ? `${prefixLeadingDigit(blockName)}__${componentName}`
-      : prefixLeadingDigit(blockName)
+      ? `${prefixLeadingDigit(blockName, '_')}__${componentName}`
+      : prefixLeadingDigit(blockName, '_')
   } else {
     return componentName
   }
@@ -134,10 +134,8 @@ const getNextId = state => {
 }
 
 const getComponentId = state => {
-  // Prefix the identifier with a character because CSS classes cannot start with a number
-  return `${useNamespace(state)}${prefixLeadingDigit(
-    getFileHash(state)
-  )}-${getNextId(state)}`
+  // Ensure identifier has prefix to avoid invalid class starting with number.
+  return `${useNamespace(state) || 'sc-'}${getFileHash(state)}-${getNextId(state)}`
 }
 
 export default t => (path, state) => {
```
chalbert: With this different approach, when using a namespace, the classname does not include `sc-` anymore. I don't know if there is a reason for that. It could be a breaking change for tools or projects that rely on having the `sc-` in the classname.
agriffis: > With this different approach, when using a namespace, the classname does not include `sc-` anymore. I don't know if there is a reason for that. It could be a breaking change for tools or projects that rely on having the `sc-` in the classname.

It wasn't consistent previously, because of conditionalizing on the digit. I doubt it's a problem.

If you feel strongly about it, that line could be `${useNamespace(state)}sc-${getFileHash(state)}-${getNextId(state)}` to always insert `sc-` regardless of the namespace, but my feeling is that the namespace serves the purpose at that point, and the extra `sc-` is just noise.
chalbert: Not being a maintainer, I'm not too sure of the impact. What I know is that jest-styled-components uses `sc-` to locate the class ― it was at the root of this bug as it was sometimes missing. So I guess it would be safer to keep `sc-` even if it does impose a bit of noise.
agriffis: Sure, that makes sense. I just updated my diff above.

Main thing I'd like to avoid is extra complexity. There are two kinds of prefixing here: (1) for eyeballing and tools, (2) to avoid invalid class names. These different reasons were conflated by the original code. Rather than make that code more complex to cover more cases, it's better to tease them apart.

If you like it, please feel free to update your PR to use this approach.

Patch:
diff --git a/src/visitors/displayNameAndId.js b/src/visitors/displayNameAndId.js
index 42c1c1f..00c1289 100644
--- a/src/visitors/displayNameAndId.js
+++ b/src/visitors/displayNameAndId.js
@@ -135,9 +135,7 @@ const getNextId = state => {
 
 const getComponentId = state => {
   // Prefix the identifier with a character because CSS classes cannot start with a number
-  return `${useNamespace(state)}${prefixLeadingDigit(
-    getFileHash(state)
-  )}-${getNextId(state)}`
+  return `${useNamespace(state)}sc-${getFileHash(state)}-${getNextId(state)}`
 }
 
 export default t => (path, state) => {


Test patch:
diff --git a/test/fixtures/add-identifier-with-top-level-import-paths-and-named-import/output.js b/test/fixtures/add-identifier-with-top-level-import-paths-and-named-import/output.js
index 71c6904..968d01b 100644
--- a/test/fixtures/add-identifier-with-top-level-import-paths-and-named-import/output.js
+++ b/test/fixtures/add-identifier-with-top-level-import-paths-and-named-import/output.js
@@ -1,27 +1,27 @@
 import { styled } from '@example/example';
 const Test = styled.div.withConfig({
   displayName: "Test",
-  componentId: "elhbfv-0"
+  componentId: "sc-elhbfv-0"
 })`width:100%;`;
 const Test2 = true ? styled.div.withConfig({
   displayName: "Test2",
-  componentId: "elhbfv-1"
+  componentId: "sc-elhbfv-1"
 })`` : styled.div.withConfig({
   displayName: "Test2",
-  componentId: "elhbfv-2"
+  componentId: "sc-elhbfv-2"
 })``;
 const styles = {
   One: styled.div.withConfig({
     displayName: "One",
-    componentId: "elhbfv-3"
+    componentId: "sc-elhbfv-3"
   })``
 };
 let Component;
 Component = styled.div.withConfig({
   displayName: "Component",
-  componentId: "elhbfv-4"
+  componentId: "sc-elhbfv-4"
 })``;
 const WrappedComponent = styled(Inner).withConfig({
   displayName: "WrappedComponent",
-  componentId: "elhbfv-5"
+  componentId: "sc-elhbfv-5"
 })``;
diff --git a/test/fixtures/annotate-styled-calls-with-pure-comments/output.js b/test/fixtures/annotate-styled-calls-with-pure-comments/output.js
index da16001..8b81f90 100644
--- a/test/fixtures/annotate-styled-calls-with-pure-comments/output.js
+++ b/test/fixtures/annotate-styled-calls-with-pure-comments/output.js
@@ -1,43 +1,43 @@
 import styled from 'styled-components';
 const Test = /*#__PURE__*/styled.div.withConfig({
   displayName: "code__Test",
-  componentId: "u20i28-0"
+  componentId: "sc-u20i28-0"
 })(["width:100%;"]);
 const Test2 = /*#__PURE__*/styled('div').withConfig({
   displayName: "code__Test2",
-  componentId: "u20i28-1"
+  componentId: "sc-u20i28-1"
 })([""]);
 const Test3 = true ? styled.div.withConfig({
   displayName: "code__Test3",
-  componentId: "u20i28-2"
+  componentId: "sc-u20i28-2"
 })([""]) : styled.div.withConfig({
   displayName: "code__Test3",
-  componentId: "u20i28-3"
+  componentId: "sc-u20i28-3"
 })([""]);
 const styles = {
   One: styled.div.withConfig({
     displayName: "code__One",
-    componentId: "u20i28-4"
+    componentId: "sc-u20i28-4"
   })([""])
 };
 let Component;
 Component = styled.div.withConfig({
   displayName: "code__Component",
-  componentId: "u20i28-5"
+  componentId: "sc-u20i28-5"
 })([""]);
 const WrappedComponent = /*#__PURE__*/styled(Inner).withConfig({
   displayName: "code__WrappedComponent",
-  componentId: "u20i28-6"
+  componentId: "sc-u20i28-6"
 })([""]);
 const StyledObjectForm = /*#__PURE__*/styled.div.withConfig({
   displayName: "code__StyledObjectForm",
-  componentId: "u20i28-7"
+  componentId: "sc-u20i28-7"
 })({
   color: red
 });
 const StyledFunctionForm = /*#__PURE__*/styled.div.withConfig({
   displayName: "code__StyledFunctionForm",
-  componentId: "u20i28-8"
+  componentId: "sc-u20i28-8"
 })(p => ({
   color: p.color || 'red'
 }));
diff --git a/test/fixtures/minify-single-line-comments-with-interpolations/output.js b/test/fixtures/minify-single-line-comments-with-interpolations/output.js
index 976310b..db4b692 100644
--- a/test/fixtures/minify-single-line-comments-with-interpolations/output.js
+++ b/test/fixtures/minify-single-line-comments-with-interpolations/output.js
@@ -1,29 +1,29 @@
 import styled from 'styled-components';
 const Test1 = styled.div.withConfig({
   displayName: "code__Test1",
-  componentId: "kc0mjf-0"
+  componentId: "sc-kc0mjf-0"
 })(["width:100%;"]);
 const Test2 = styled.div.withConfig({
   displayName: "code__Test2",
-  componentId: "kc0mjf-1"
+  componentId: "sc-kc0mjf-1"
 })(["width:100%;"]);
 const Test3 = styled.div.withConfig({
   displayName: "code__Test3",
-  componentId: "kc0mjf-2"
+  componentId: "sc-kc0mjf-2"
 })(["width:100%;", ";"], 'red');
 const Test4 = styled.div.withConfig({
   displayName: "code__Test4",
-  componentId: "kc0mjf-3"
+  componentId: "sc-kc0mjf-3"
 })(["width:100%;"]);
 const Test5 = styled.div.withConfig({
   displayName: "code__Test5",
-  componentId: "kc0mjf-4"
+  componentId: "sc-kc0mjf-4"
 })(["width:100%;"]);
 const Test6 = styled.div.withConfig({
   displayName: "code__Test6",
-  componentId: "kc0mjf-5"
+  componentId: "sc-kc0mjf-5"
 })(["background:url(\"https://google.com\");width:100%;", " "], 'green');
 const Test7 = styled.div.withConfig({
   displayName: "code__Test7",
-  componentId: "kc0mjf-6"
+  componentId: "sc-kc0mjf-6"
 })(["background:url(\"https://google.com\");width:", ";", "  height:", ";"], p => p.props.width, 'green', p => p.props.height);
diff --git a/test/fixtures/track-the-imported-variable/output.js b/test/fixtures/track-the-imported-variable/output.js
index 1a57590..773e474 100644
--- a/test/fixtures/track-the-imported-variable/output.js
+++ b/test/fixtures/track-the-imported-variable/output.js
@@ -1,27 +1,27 @@
 import s from "styled-components";
 const Test = s.div.withConfig({
   displayName: "Test",
-  componentId: "wyof43-0"
+  componentId: "sc-wyof43-0"
 })`width:100%;`;
 const Test2 = true ? s.div.withConfig({
   displayName: "Test2",
-  componentId: "wyof43-1"
+  componentId: "sc-wyof43-1"
 })`` : s.div.withConfig({
   displayName: "Test2",
-  componentId: "wyof43-2"
+  componentId: "sc-wyof43-2"
 })``;
 const styles = {
   One: s.div.withConfig({
     displayName: "One",
-    componentId: "wyof43-3"
+    componentId: "sc-wyof43-3"
   })``
 };
 let Component;
 Component = s.div.withConfig({
   displayName: "Component",
-  componentId: "wyof43-4"
+  componentId: "sc-wyof43-4"
 })``;
 const WrappedComponent = s(Inner).withConfig({
   displayName: "WrappedComponent",
-  componentId: "wyof43-5"
+  componentId: "sc-wyof43-5"
 })``;