Merge pull request #1755 from styled-components/fix-1715

fix generated component id in non-babel scenario
styled-components · Jun 20, 2018 · 5bc17dc · 5bc17dc
2 parents 7e71f6c + bf4ed48
commit 5bc17dc
Show file tree

Hide file tree

Showing 5 changed files with 69 additions and 23 deletions.
diff --git a/.eslintrc b/.eslintrc
@@ -4,29 +4,48 @@
   "globals": {
     "__SERVER__": true
   },
-  "plugins": ["flowtype", "flowtype-errors", "prettier"],
+  "plugins": [
+    "flowtype",
+    "flowtype-errors",
+    "prettier"
+  ],
   "rules": {
     "curly": [2, "multi-line"],
     "class-methods-use-this": 0,
     "symbol-description": 0,
-    "no-unused-vars": [2, { "varsIgnorePattern": "^_+$" }],
+    "no-unused-vars": [
+      2,
+      {
+        "varsIgnorePattern": "^_+$"
+      }
+    ],
     "import/no-extraneous-dependencies": 0,
     "no-confusing-arrow": 0,
     "no-else-return": 0,
     "react/sort-comp": 0,
     "react/jsx-filename-extension": 0,
+    "react/prefer-stateless-function": 0,
+    "react/no-multi-comp": 0,
     "react/require-default-props": 0,
     "flowtype-errors/show-errors": 2,
     "no-prototype-builtins": 0,
     "no-duplicate-imports": 0,
     "flowtype/require-valid-file-annotation": [
       2,
       "always",
-      { "annotationStyle": "line" }
+      {
+        "annotationStyle": "line"
+      }
+    ],
+    "flowtype/boolean-style": [
+      2,
+      "boolean"
     ],
-    "flowtype/boolean-style": [2, "boolean"],
     "flowtype/no-dupe-keys": 2,
-    "flowtype/union-intersection-spacing": [2, "always"],
+    "flowtype/union-intersection-spacing": [
+      2,
+      "always"
+    ],
     "flowtype/object-type-delimiter": 2,
     "prettier/prettier": 2,
     "no-restricted-syntax": [
@@ -50,4 +69,4 @@
       "experimentalObjectRestSpread": true
     }
   }
-}
+}
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -8,6 +8,8 @@ _The format is based on [Keep a Changelog](http://keepachangelog.com/) and this
 
 * Fixed a regression when extending a `styled(StyledComponent)` introduced in 3.3.0, by @probablyup (see #1819)
 
+* Adjust how displayName is generated when not using Babel to properly preserve a displayName passed via `withConfig`, by @probablyup (see #1755)
+
 ## [v3.3.2] - 2018-06-04
 
 * Allow non-plain objects as `ThemeProvider` themes, by @phyllisstein (see #1780)

diff --git a/package.json b/package.json
@@ -195,4 +195,4 @@
     "type": "opencollective",
     "url": "https://opencollective.com/styled-components"
   }
-}
+}
diff --git a/src/models/StyledComponent.js b/src/models/StyledComponent.js
@@ -37,22 +37,16 @@ export default (ComponentStyle: Function, constructWithOptions: Function) => {
     const displayName =
       typeof _displayName !== 'string' ? 'sc' : escape(_displayName)
 
-    let componentId
-
     /**
-     * only fall back to hashing the component injection order if
-     * a proper displayName isn't provided by the babel plugin
+     * This ensures uniqueness if two components happen to share
+     * the same displayName.
      */
-    if (!_displayName) {
-      const nr = (identifiers[displayName] || 0) + 1
-      identifiers[displayName] = nr
-
-      componentId = `${displayName}-${ComponentStyle.generateName(
-        displayName + nr
-      )}`
-    } else {
-      componentId = `${displayName}-${ComponentStyle.generateName(displayName)}`
-    }
+    const nr = (identifiers[displayName] || 0) + 1
+    identifiers[displayName] = nr
+
+    const componentId = `${displayName}-${ComponentStyle.generateName(
+      displayName + nr
+    )}`
 
     return parentComponentId !== undefined
       ? `${parentComponentId}-${componentId}`
@@ -270,7 +264,7 @@ export default (ComponentStyle: Function, constructWithOptions: Function) => {
     const styledComponentId =
       options.displayName && options.componentId
         ? `${escape(options.displayName)}-${options.componentId}`
-        : componentId
+        : options.componentId || componentId
 
     const componentStyle = new ComponentStyle(
       extendingRules === undefined ? rules : extendingRules.concat(rules),

diff --git a/src/test/basic.test.js b/src/test/basic.test.js
@@ -115,7 +115,7 @@ describe('basic', () => {
 
   it('should allow you to pass in style objects', () => {
     const Comp = styled.div({
-      color: 'blue'
+      color: 'blue',
     })
     shallow(<Comp />)
     expectCSSMatches('.sc-a {} .b { color:blue; }')
@@ -258,5 +258,36 @@ describe('basic', () => {
         InnerComponent.componentStyle
       )
     })
+
+    it('generates unique classnames when not using babel', () => {
+      const Named1 = styled.div.withConfig({ displayName: 'Name' })`
+        color: blue;
+      `
+
+      const Named2 = styled.div.withConfig({ displayName: 'Name' })`
+        color: red;
+      `
+
+      expect(Named1.styledComponentId).not.toBe(Named2.styledComponentId)
+    })
+
+    it('honors a passed componentId', () => {
+      const Named1 = styled.div.withConfig({
+        componentId: 'foo',
+        displayName: 'Name',
+      })`
+        color: blue;
+      `
+
+      const Named2 = styled.div.withConfig({
+        componentId: 'bar',
+        displayName: 'Name',
+      })`
+        color: red;
+      `
+
+      expect(Named1.styledComponentId).toBe('Name-foo')
+      expect(Named2.styledComponentId).toBe('Name-bar')
+    })
   })
 })