Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add .extend and .withComponent deterministic ID generation #1044

Merged
merged 7 commits into from
Jul 30, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@ All notable changes to this project will be documented in this file. If a contri
- Refactor variable in generateAlphabeticName.js (see [#1040](https://github.com/styled-components/styled-components/pull/1040))
- Enable the Node environment for SSR tests, switch some output verification to snapshot testing (see [#1023](https://github.com/styled-components/styled-components/pull/1023))

- Add .extend and .withComponent deterministic ID generation (see [#1044](https://github.com/styled-components/styled-components/pull/1044))

## [v2.1.0] - 2017-06-15

- Added missing v2.0 APIs to TypeScript typings, thanks to [@patrick91](https://github.com/patrick91), [@igorbek](https://github.com/igorbek) (see [#837](https://github.com/styled-components/styled-components/pull/837), [#882](https://github.com/styled-components/styled-components/pull/882))
Expand Down
34 changes: 24 additions & 10 deletions src/models/StyledComponent.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ const multiDashRegex = /--+/g
export default (ComponentStyle: Function, constructWithOptions: Function) => {
/* We depend on components having unique IDs */
const identifiers = {}
const generateId = (_displayName: string) => {
const generateId = (_displayName: string, parentComponentId: string) => {
const displayName = typeof _displayName !== 'string' ?
'sc' : _displayName
.replace(escapeRegex, '-') // Replace all possible CSS selectors
Expand All @@ -32,7 +32,10 @@ export default (ComponentStyle: Function, constructWithOptions: Function) => {
identifiers[displayName] = nr

const hash = ComponentStyle.generateName(displayName + nr)
return `${displayName}-${hash}`
const componentId = `${displayName}-${hash}`
return parentComponentId !== undefined
? `${parentComponentId}-${componentId}`
: componentId
}

class BaseStyledComponent extends AbstractStyledComponent {
Expand Down Expand Up @@ -175,7 +178,7 @@ export default (ComponentStyle: Function, constructWithOptions: Function) => {
) => {
const {
displayName = isTag(target) ? `styled.${target}` : `Styled(${getComponentName(target)})`,
componentId = generateId(options.displayName),
componentId = generateId(options.displayName, options.parentComponentId),
ParentComponent = BaseStyledComponent,
rules: extendingRules,
attrs,
Expand Down Expand Up @@ -208,26 +211,37 @@ export default (ComponentStyle: Function, constructWithOptions: Function) => {
static target = target

static withComponent(tag) {
const { displayName: _, componentId: __, ...optionsToCopy } = options
const newOptions = { ...optionsToCopy, ParentComponent: StyledComponent }
const { componentId: previousComponentId, ...optionsToCopy } = options

const newComponentId =
previousComponentId &&
`${previousComponentId}-${isTag(tag) ? tag : getComponentName(tag)}`
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we should extract this logic into a small function at the top of this file, now that it's used twice?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not exactly the same, so I prefer to keep it separated.


const newOptions = {
...optionsToCopy,
componentId: newComponentId,
ParentComponent: StyledComponent,
}

return createStyledComponent(tag, newOptions, rules)
}

static get extend() {
const {
displayName: _,
componentId: __,
rules: rulesFromOptions,
componentId: parentComponentId,
...optionsToCopy
} = options

const newRules = rulesFromOptions === undefined
? rules
: rulesFromOptions.concat(rules)
const newRules =
rulesFromOptions === undefined
? rules
: rulesFromOptions.concat(rules)

const newOptions = {
...optionsToCopy,
rules: newRules,
parentComponentId,
ParentComponent: StyledComponent,
}

Expand Down
24 changes: 24 additions & 0 deletions src/test/expanded-api.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,30 @@ describe('expanded api', () => {
expect(Comp2.styledComponentId).toBe('Comp2-OMGLOL')
expect(shallow(<Comp2 />).prop('className')).toMatch(/Comp2-OMGLOL/)
})

it('should work with `.extend`', () => {
const Comp = styled.div.withConfig({ displayName: 'Comp', componentId: 'LOLOMG' })`
color: blue;
`
const Comp2 = Comp.extend`
color: ${'red'};
background: ${props => props.bg};
`
expect(Comp.styledComponentId).toBe('Comp-LOLOMG')
expect(shallow(<Comp />).prop('className')).toMatch(/Comp-LOLOMG/)
expect(Comp2.styledComponentId).toBe('LOLOMG-Comp-a')
expect(shallow(<Comp2 bg="red" />).prop('className')).toMatch(/LOLOMG-Comp-a/)
})

it('should work with `.withComponent`', () => {
const Dummy = () => null
const Comp = styled.div.withConfig({ displayName: 'Comp', componentId: 'OMGLOL' })``.withComponent('h1')
const Comp2 = styled.div.withConfig({ displayName: 'Comp2', componentId: 'OMFG' })``.withComponent(Dummy)
expect(Comp.styledComponentId).toBe('Comp-OMGLOL-h1')
expect(shallow(<Comp />).prop('className')).toMatch(/Comp-OMGLOL-h1/)
expect(Comp2.styledComponentId).toBe('Comp2-OMFG-Dummy')
expect(shallow(<Comp2 />).prop('className')).toMatch(/Comp2-OMFG-Dummy/)
})
})

describe('chaining', () => {
Expand Down