From 97a637a038521e822568362849edd05652e557a3 Mon Sep 17 00:00:00 2001 From: Evan Jacobs Date: Sat, 8 Jun 2019 19:02:23 -0500 Subject: [PATCH] add seal fn back to ServerStyleSheet for backward compat (#2581) * add seal fn back to ServerStyleSheet for backward compat * Make ServerStyleSheet sealed error more precise - collectStyles could very well be used for multiple elements although that's a rare use case - getStyleTags and getStyleElement are safe to be called again, unless interleaveWithNodeStream was called already - interleaveWithNodeStream is not safe to be called again * add tests verifying the error scenarios, allow getStyleTags during streaming --- .../src/models/ServerStyleSheet.js | 40 +++-- .../src/test/__snapshots__/ssr.test.js.snap | 51 ++++++ .../styled-components/src/test/ssr.test.js | 151 +++++++++++++++++- src/utils/errors.md | 7 +- 4 files changed, 228 insertions(+), 21 deletions(-) diff --git a/packages/styled-components/src/models/ServerStyleSheet.js b/packages/styled-components/src/models/ServerStyleSheet.js index a8f4c284..011e745c 100644 --- a/packages/styled-components/src/models/ServerStyleSheet.js +++ b/packages/styled-components/src/models/ServerStyleSheet.js @@ -2,9 +2,7 @@ /* eslint-disable no-underscore-dangle */ import React from 'react'; - import { IS_BROWSER, SC_ATTR, SC_ATTR_VERSION, SC_VERSION } from '../constants'; - import StyledError from '../utils/error'; import getNonce from '../utils/nonce'; import StyleSheet from '../sheet'; @@ -15,6 +13,8 @@ declare var __SERVER__: boolean; const CLOSING_TAG_R = /^\s*<\/[a-z]/i; export default class ServerStyleSheet { + isStreaming: boolean; + sheet: StyleSheet; sealed: boolean; @@ -24,25 +24,36 @@ export default class ServerStyleSheet { this.sealed = false; } + _emitSheetCSS = (): string => { + const css = this.sheet.toString(); + const nonce = getNonce(); + const attrs = [nonce && `nonce="${nonce}"`, SC_ATTR, `${SC_ATTR_VERSION}="${SC_VERSION}"`]; + const htmlAttr = attrs.filter(Boolean).join(' '); + + return ``; + }; + collectStyles(children: any) { if (this.sealed) { throw new StyledError(2); } - this.sealed = true; return {children}; } getStyleTags = (): string => { - const css = this.sheet.toString(); - const nonce = getNonce(); - const attrs = [nonce && `nonce="${nonce}"`, SC_ATTR, `${SC_ATTR_VERSION}="${SC_VERSION}"`]; + if (this.sealed) { + throw new StyledError(2); + } - const htmlAttr = attrs.filter(Boolean).join(' '); - return ``; + return this._emitSheetCSS(); }; getStyleElement = () => { + if (this.sealed) { + throw new StyledError(2); + } + const props = { [SC_ATTR]: '', [SC_ATTR_VERSION]: SC_VERSION, @@ -62,20 +73,25 @@ export default class ServerStyleSheet { interleaveWithNodeStream(input: any) { if (!__SERVER__ || IS_BROWSER) { throw new StyledError(3); + } else if (this.sealed) { + throw new StyledError(2); } + this.seal(); + // eslint-disable-next-line global-require const { Readable, Transform } = require('stream'); const readableStream: Readable = input; - const { sheet, getStyleTags } = this; + const { sheet, _emitSheetCSS } = this; const transformer = new Transform({ transform: function appendStyleChunks(chunk, /* encoding */ _, callback) { // Get the chunk and retrieve the sheet's CSS as an HTML chunk, // then reset its rules so we get only new ones for the next chunk const renderedHtml = chunk.toString(); - const html = getStyleTags(); + const html = _emitSheetCSS(); + sheet.clearTag(); // prepend style html to chunk, unless the start of the chunk is a @@ -101,4 +117,8 @@ export default class ServerStyleSheet { return readableStream.pipe(transformer); } + + seal = () => { + this.sealed = true; + }; } diff --git a/packages/styled-components/src/test/__snapshots__/ssr.test.js.snap b/packages/styled-components/src/test/__snapshots__/ssr.test.js.snap index 27e7e534..648d7dfb 100644 --- a/packages/styled-components/src/test/__snapshots__/ssr.test.js.snap +++ b/packages/styled-components/src/test/__snapshots__/ssr.test.js.snap @@ -61,3 +61,54 @@ data-styled.g2[id=\\"sc-b\\"]{content:\\"c,\\"} "data-styled-version": "JEST_MOCK_VERSION", } `; + +exports[`ssr should throw if getStyleElement is called after interleaveWithNodeStream is called 1`] = ` +"Can't collect styles once you've consumed a \`ServerStyleSheet\`'s styles! \`ServerStyleSheet\` is a one off instance for each server-side render cycle. + +- Are you trying to reuse it across renders? +- Are you accidentally calling collectStyles twice?" +`; + +exports[`ssr should throw if getStyleElement is called after streaming is complete 1`] = ` +"

Hello SSR!

" +`; + +exports[`ssr should throw if getStyleElement is called after streaming is complete 2`] = ` +"Can't collect styles once you've consumed a \`ServerStyleSheet\`'s styles! \`ServerStyleSheet\` is a one off instance for each server-side render cycle. + +- Are you trying to reuse it across renders? +- Are you accidentally calling collectStyles twice?" +`; + +exports[`ssr should throw if getStyleTags is called after interleaveWithNodeStream is called 1`] = ` +"Can't collect styles once you've consumed a \`ServerStyleSheet\`'s styles! \`ServerStyleSheet\` is a one off instance for each server-side render cycle. + +- Are you trying to reuse it across renders? +- Are you accidentally calling collectStyles twice?" +`; + +exports[`ssr should throw if getStyleTags is called after streaming is complete 1`] = ` +"

Hello SSR!

" +`; + +exports[`ssr should throw if getStyleTags is called after streaming is complete 2`] = ` +"Can't collect styles once you've consumed a \`ServerStyleSheet\`'s styles! \`ServerStyleSheet\` is a one off instance for each server-side render cycle. + +- Are you trying to reuse it across renders? +- Are you accidentally calling collectStyles twice?" +`; + +exports[`ssr should throw if interleaveWithNodeStream is called twice 1`] = ` +"Can't collect styles once you've consumed a \`ServerStyleSheet\`'s styles! \`ServerStyleSheet\` is a one off instance for each server-side render cycle. + +- Are you trying to reuse it across renders? +- Are you accidentally calling collectStyles twice?" +`; diff --git a/packages/styled-components/src/test/ssr.test.js b/packages/styled-components/src/test/ssr.test.js index 4fd6b23b..172fafdf 100644 --- a/packages/styled-components/src/test/ssr.test.js +++ b/packages/styled-components/src/test/ssr.test.js @@ -142,7 +142,8 @@ describe('ssr', () => { `; const sheet = new ServerStyleSheet(); - const html = renderToString( + + renderToString( sheet.collectStyles( @@ -150,6 +151,7 @@ describe('ssr', () => { ) ); + const element = sheet.getStyleElement(); expect(element.props.dangerouslySetInnerHTML).toBeDefined(); @@ -169,7 +171,8 @@ describe('ssr', () => { `; const sheet = new ServerStyleSheet(); - const html = renderToString( + + renderToString( sheet.collectStyles( Hello SSR! @@ -236,11 +239,6 @@ describe('ssr', () => { color: green; `; - // These create a long chunk of (hopefully) uninterrupted HTML - const elements = new Array(100) - .fill(0) - .map((_, i) =>
*************************
); - // This is the result of the above const expectedElements = '
*************************
'.repeat(100); @@ -284,7 +282,7 @@ describe('ssr', () => { const jsx = sheet.collectStyles(null); const stream = sheet.interleaveWithNodeStream(renderToNodeStream(jsx)); - return new Promise((resolve, reject) => { + return new Promise(resolve => { stream.on('data', () => {}); stream.on('error', err => { @@ -339,4 +337,141 @@ describe('ssr', () => { stream.on('error', reject); }); }); + + it('should throw if interleaveWithNodeStream is called twice', () => { + const Component = createGlobalStyle` + body { background: papayawhip; } + `; + const Heading = styled.h1` + color: red; + `; + + const sheet = new ServerStyleSheet(); + const jsx = sheet.collectStyles( + + + Hello SSR! + + ); + + expect(() => + sheet.interleaveWithNodeStream(sheet.interleaveWithNodeStream(renderToNodeStream(jsx))) + ).toThrowErrorMatchingSnapshot(); + }); + + it('should throw if getStyleTags is called after interleaveWithNodeStream is called', () => { + const Component = createGlobalStyle` + body { background: papayawhip; } + `; + const Heading = styled.h1` + color: red; + `; + + const sheet = new ServerStyleSheet(); + + const jsx = sheet.collectStyles( + + + Hello SSR! + + ); + + sheet.interleaveWithNodeStream(renderToNodeStream(jsx)); + + expect(sheet.getStyleTags).toThrowErrorMatchingSnapshot(); + }); + + it('should throw if getStyleElement is called after interleaveWithNodeStream is called', () => { + const Component = createGlobalStyle` + body { background: papayawhip; } + `; + const Heading = styled.h1` + color: red; + `; + + const sheet = new ServerStyleSheet(); + + const jsx = sheet.collectStyles( + + + Hello SSR! + + ); + + sheet.interleaveWithNodeStream(renderToNodeStream(jsx)); + + expect(sheet.getStyleElement).toThrowErrorMatchingSnapshot(); + }); + + it('should throw if getStyleTags is called after streaming is complete', () => { + const Component = createGlobalStyle` + body { background: papayawhip; } + `; + const Heading = styled.h1` + color: red; + `; + + const sheet = new ServerStyleSheet(); + const jsx = sheet.collectStyles( + + + Hello SSR! + + ); + const stream = sheet.interleaveWithNodeStream(renderToNodeStream(jsx)); + + return new Promise((resolve, reject) => { + let received = ''; + + stream.on('data', chunk => { + received += chunk; + }); + + stream.on('end', () => { + expect(received).toMatchSnapshot(); + expect(sheet.sealed).toBe(true); + expect(sheet.getStyleTags).toThrowErrorMatchingSnapshot(); + + resolve(); + }); + + stream.on('error', reject); + }); + }); + + it('should throw if getStyleElement is called after streaming is complete', () => { + const Component = createGlobalStyle` + body { background: papayawhip; } + `; + const Heading = styled.h1` + color: red; + `; + + const sheet = new ServerStyleSheet(); + const jsx = sheet.collectStyles( + + + Hello SSR! + + ); + const stream = sheet.interleaveWithNodeStream(renderToNodeStream(jsx)); + + return new Promise((resolve, reject) => { + let received = ''; + + stream.on('data', chunk => { + received += chunk; + }); + + stream.on('end', () => { + expect(received).toMatchSnapshot(); + expect(sheet.sealed).toBe(true); + expect(sheet.getStyleElement).toThrowErrorMatchingSnapshot(); + + resolve(); + }); + + stream.on('error', reject); + }); + }); }); diff --git a/src/utils/errors.md b/src/utils/errors.md index d63ff227..98330097 100644 --- a/src/utils/errors.md +++ b/src/utils/errors.md @@ -6,10 +6,11 @@ Cannot create styled-component for component: %s. ## 2 -Can't collect styles once you've consumed a `ServerStyleSheet`'s styles! `ServerStyleSheet` is a one off instance for each server-side render cycle. +Can't call method, once `interleaveWithNodeStream` is used, since it +will split the underlying styles into multiple parts. -- Are you trying to reuse it across renders? -- Are you accidentally calling collectStyles twice? +- Are you trying to call `interleaveWithNodeStream` twice? +- Are you calling `getStyleTags`, `getStyleElement`, or `collectStyles` after it? ## 3