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

backport dev SSR classname mismatch fix from canary #2701

Merged
merged 2 commits into from
Aug 21, 2019
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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@ _The format is based on [Keep a Changelog](http://keepachangelog.com/) and this

## Unreleased

- Backport fix for SSR classname mismatches in development mode for some environments like next.js (see [#2701](https://github.com/styled-components/styled-components/pull/2701))

## [v4.3.2] - 2019-06-19

- Fix usage of the "css" prop with the styled-components babel macro (relevant to CRA 2.x users), by [@jamesknelson](http://github.com/jamesknelson) (see [#2633](https://github.com/styled-components/styled-components/issues/2633))
Expand Down
13 changes: 5 additions & 8 deletions packages/styled-components/src/models/ComponentStyle.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,6 @@ import { IS_BROWSER } from '../constants';

import type { Attrs, RuleSet } from '../types';

const isHMREnabled =
process.env.NODE_ENV !== 'production' && typeof module !== 'undefined' && module.hot;

/* combines hashStr (murmurhash) and nameGenerator for convenience */
const hasher = (str: string): string => generateAlphabeticName(hashStr(str));

Expand All @@ -31,7 +28,7 @@ export default class ComponentStyle {

constructor(rules: RuleSet, attrs: Attrs, componentId: string) {
this.rules = rules;
this.isStatic = !isHMREnabled && isStaticRules(rules, attrs);
this.isStatic = process.env.NODE_ENV === 'production' && isStaticRules(rules, attrs);
this.componentId = componentId;

if (!StyleSheet.master.hasId(componentId)) {
Expand All @@ -40,10 +37,10 @@ export default class ComponentStyle {
}

/*
* Flattens a rule set into valid CSS
* Hashes it, wraps the whole chunk in a .hash1234 {}
* Returns the hash to be injected on render()
* */
* Flattens a rule set into valid CSS
* Hashes it, wraps the whole chunk in a .hash1234 {}
* Returns the hash to be injected on render()
* */
generateAndInjectStyles(executionContext: Object, styleSheet: StyleSheet) {
const { isStatic, componentId, lastClassName } = this;
if (
Expand Down
80 changes: 43 additions & 37 deletions packages/styled-components/src/test/staticCaching.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,32 +8,6 @@ describe('static style caching', () => {
styled = resetStyled();
});

it('should mark styles without any functions as static', () => {
const TOP_AS_NUMBER = 10;
const FONT_SIZE_NUMBER = 14;

const Comp = styled.div`
color: purple;
font-size: ${FONT_SIZE_NUMBER}px
position: absolute;
top: ${TOP_AS_NUMBER}
`;

expect(Comp.componentStyle.isStatic).toEqual(true);
});

it('should mark styles with a nested styled component as static', () => {
const NestedComp = styled.div``;

const Comp = styled.div`
${NestedComp} {
color: purple;
}
`;

expect(Comp.componentStyle.isStatic).toEqual(true);
});

it('should mark styles with a dynamic style as not static', () => {
const Comp = styled.div`
color: ${props => props.color};
Expand All @@ -42,17 +16,6 @@ describe('static style caching', () => {
expect(Comp.componentStyle.isStatic).toEqual(false);
});

it('should mark components with numeric attriutes as static', () => {
const Comp = styled.div.attrs({
style: {
color: 'purple',
},
height: 100,
})``;

expect(Comp.componentStyle.isStatic).toEqual(true);
});

it('should mark components with dynamic attributes as not static', () => {
const Comp = styled.div.attrs({
style: props => ({
Expand All @@ -62,4 +25,47 @@ describe('static style caching', () => {

expect(Comp.componentStyle.isStatic).toEqual(false);
});

describe('production mode', () => {
beforeEach(() => {
process.env.NODE_ENV = 'production';
});

it('should mark styles without any functions as static', () => {
const TOP_AS_NUMBER = 10;
const FONT_SIZE_NUMBER = 14;

const Comp = styled.div`
color: purple;
font-size: ${FONT_SIZE_NUMBER}px
position: absolute;
top: ${TOP_AS_NUMBER}
`;

expect(Comp.componentStyle.isStatic).toEqual(true);
});

it('should mark styles with a nested styled component as static', () => {
const NestedComp = styled.div``;

const Comp = styled.div`
${NestedComp} {
color: purple;
}
`;

expect(Comp.componentStyle.isStatic).toEqual(true);
});

it('should mark components with numeric attributes as static', () => {
const Comp = styled.div.attrs({
style: {
color: 'purple',
},
height: 100,
})``;

expect(Comp.componentStyle.isStatic).toEqual(true);
});
});
});