From 966a012a56493985b7f75821de9d7afd1763b297 Mon Sep 17 00:00:00 2001 From: Christopher Pappas Date: Fri, 22 Nov 2024 18:53:38 -0800 Subject: [PATCH] feat(react-18): Add streaming implementation --- src/Server/config.ts | 1 + src/Server/setup_sharify.ts | 1 + src/System/Boot.tsx | 19 ++- src/System/Router/Utils/collectAssets.tsx | 124 +++++++++++------- src/System/Router/Utils/renderToStream.tsx | 74 +++++++++++ .../__tests__/serverRouter.jest.enzyme.tsx | 18 ++- src/System/Router/renderServerApp.tsx | 39 +++++- src/System/Router/serverRouter.tsx | 8 +- src/Typings/sharify.d.ts | 1 + webpack/bundleSplitting.js | 9 ++ 10 files changed, 219 insertions(+), 75 deletions(-) create mode 100644 src/System/Router/Utils/renderToStream.tsx diff --git a/src/Server/config.ts b/src/Server/config.ts index 5a7ba242224..42602a6265d 100644 --- a/src/Server/config.ts +++ b/src/Server/config.ts @@ -51,6 +51,7 @@ export const EDITORIAL_PATHS: any = export const ENABLE_CONVERSATIONS_MESSAGES_AUTO_REFRESH: any = true export const ENABLE_NEW_AUCTIONS_FILTER: any = false export const ENABLE_QUERY_BATCHING: any = false +export const ENABLE_SSR_STREAMING: any = false export const ENABLE_WEB_CRAWLING: any = false export const ENABLE_WEB_VITALS_LOGGING: any = false export const GRAPHQL_CACHE_TTL: any = 1000000000 diff --git a/src/Server/setup_sharify.ts b/src/Server/setup_sharify.ts index 13ee3dda725..1a41455d379 100644 --- a/src/Server/setup_sharify.ts +++ b/src/Server/setup_sharify.ts @@ -46,6 +46,7 @@ sharify.data = _.extend( "ENABLE_CONVERSATIONS_MESSAGES_AUTO_REFRESH", "ENABLE_NEW_AUCTIONS_FILTER", "ENABLE_QUERY_BATCHING", + "ENABLE_SSR_STREAMING", "ENABLE_WEB_CRAWLING", "ENABLE_WEB_VITALS_LOGGING", "ERROR", diff --git a/src/System/Boot.tsx b/src/System/Boot.tsx index 2d93256bd88..165909a20e6 100644 --- a/src/System/Boot.tsx +++ b/src/System/Boot.tsx @@ -35,12 +35,11 @@ export interface BootProps extends React.PropsWithChildren { const { GlobalStyles } = injectGlobalStyles() -export const Boot: React.FC>> = track( - undefined, - { - dispatch: Events.postEvent, - } -)((props: BootProps) => { +export const Boot: React.FC +>> = track(undefined, { + dispatch: Events.postEvent, +})((props: BootProps) => { /** * Let our end-to-end tests know that the app is hydrated and ready to go; and * if in prod, initialize Sentry. @@ -82,7 +81,6 @@ export const Boot: React.FC - {children} @@ -100,10 +98,9 @@ export const Boot: React.FC> = ({ - children, - environment, -}) => { +const EnvironmentProvider: FC> = ({ children, environment }) => { if (process.env.NODE_ENV === "test") return <>{children} return ( diff --git a/src/System/Router/Utils/collectAssets.tsx b/src/System/Router/Utils/collectAssets.tsx index 58e8fb01cdf..7004d064e5d 100644 --- a/src/System/Router/Utils/collectAssets.tsx +++ b/src/System/Router/Utils/collectAssets.tsx @@ -9,6 +9,9 @@ import RelayServerSSR, { SSRCache, } from "react-relay-network-modern-ssr/lib/server" import { RelayNetworkLayerResponse } from "react-relay-network-modern" +import { ENABLE_SSR_STREAMING } from "Server/config" +import { renderToStream } from "System/Router/Utils/renderToStream" +import { ArtsyResponse } from "Server/middleware/artsyExpress" const STATS = "loadable-stats.json" @@ -19,11 +22,13 @@ const ASSET_PATH = "/assets" interface CollectAssetsProps { ServerRouter: React.FC> relayEnvironment: Environment + res: ArtsyResponse } export const collectAssets = async ({ ServerRouter, relayEnvironment, + res, }: CollectAssetsProps) => { /** * This is the stats file generated by Force's webpack setup, via @@ -52,71 +57,88 @@ export const collectAssets = async ({ const jsx = extractor.collectChunks(sheet.collectStyles()) - const html = renderToString(jsx) - const styleTags = sheet.getStyleTags() + let stream + let html + let styleTags + + if (ENABLE_SSR_STREAMING) { + stream = renderToStream(jsx, sheet, res) + } else { + html = renderToString(jsx) + styleTags = sheet.getStyleTags() + } const relaySSRMiddleware = (relayEnvironment as any) .relaySSRMiddleware as RelayServerSSR const initialRelayData = await relaySSRMiddleware.getCache() - const initialScripts: string[] = [] - - const bundleScriptTags = extractor.getScriptTags() - - initialScripts.push( - bundleScriptTags - .split("\n") - .map(script => { - /** - * In production, prefix injected script src with CDN endpoint. - * @see https://github.com/artsy/force/blob/main/src/lib/middleware/asset.ts#L23 - */ - if (getENV("CDN_URL")) { - const scriptTagWithCDN = script.replace( - /src="\/assets/g, - `src="${assetPublicPath}` - ) - return scriptTagWithCDN - } else { - return script - } - }) - .filter(script => { - return !( + /** + * Extract scripts from loadable components + * NOTE: When streaming is enabled we need to lazily execute script extraction + */ + const extractScriptTags = () => { + const initialScripts: string[] = [] + + const bundleScriptTags = extractor.getScriptTags() + + initialScripts.push( + bundleScriptTags + .split("\n") + .map(script => { /** - * Since these files are already embedded in our main - * layout file, omit them from the scripts array. - * - * TODO: Don't include these files in the main layout - * and instead relay on dynamically including them here. - * Will require some shuffling in the jade template, - * however. + * In production, prefix injected script src with CDN endpoint. + * @see https://github.com/artsy/force/blob/main/src/lib/middleware/asset.ts#L23 */ - ( - script.includes("/assets/runtime.") || - script.includes("/assets/artsy.") || - script.includes("/assets/common-artsy.") || - script.includes("/assets/common-react.") || - script.includes("/assets/common-utility.") || - script.includes("/assets/common.") + if (getENV("CDN_URL")) { + const scriptTagWithCDN = script.replace( + /src="\/assets/g, + `src="${assetPublicPath}` + ) + return scriptTagWithCDN + } else { + return script + } + }) + .filter(script => { + return !( + /** + * Since these files are already embedded in our main + * layout file, omit them from the scripts array. + * + * TODO: Don't include these files in the main layout + * and instead relay on dynamically including them here. + * Will require some shuffling in the jade template, + * however. + */ + ( + script.includes("/assets/runtime.") || + script.includes("/assets/artsy.") || + script.includes("/assets/common-artsy.") || + script.includes("/assets/common-react.") || + script.includes("/assets/common-utility.") || + script.includes("/assets/common.") + ) ) - ) - }) - .join("\n") - ) + }) + .join("\n") + ) - initialScripts.push(` - - `) + initialScripts.push(` + + `) - const scripts = initialScripts.join("\n") + const scripts = initialScripts.join("\n") + + return scripts + } return { html, - scripts, + extractScriptTags, + stream, styleTags, } } diff --git a/src/System/Router/Utils/renderToStream.tsx b/src/System/Router/Utils/renderToStream.tsx new file mode 100644 index 00000000000..6f6df69500e --- /dev/null +++ b/src/System/Router/Utils/renderToStream.tsx @@ -0,0 +1,74 @@ +import { renderToPipeableStream } from "react-dom/server" +import { ArtsyResponse } from "Server/middleware/artsyExpress" +import { Transform } from "stream" + +const STREAM_TIMEOUT = 10000 + +export function renderToStream(jsx, sheet, res: ArtsyResponse) { + let didError = false + + const stream = new Transform({ + objectMode: true, + transform(chunk, encoding, callback) { + const renderedHtml = + chunk instanceof Uint8Array + ? Buffer.from(chunk).toString(encoding || "utf8") + : Buffer.isBuffer(chunk) + ? chunk.toString(encoding || "utf8") + : chunk + + const styledCSS = sheet._emitSheetCSS() + const CLOSING_TAG_R = /<\/[a-z]*>/i + + sheet.instance.clearTag() + + // Inject CSS into HTML + if (/<\/head>/.test(renderedHtml)) { + const replacedHtml = renderedHtml.replace( + "", + `${styledCSS}` + ) + this.push(replacedHtml) + } else if (CLOSING_TAG_R.test(renderedHtml)) { + const execResult = CLOSING_TAG_R.exec(renderedHtml) as RegExpExecArray + const endOfClosingTag = execResult.index + execResult[0].length + const before = renderedHtml.slice(0, endOfClosingTag) + const after = renderedHtml.slice(endOfClosingTag) + + this.push(before + styledCSS + after) + } else { + this.push(styledCSS + renderedHtml) + } + + callback() + }, + }) + + let streamTimeout: NodeJS.Timeout + + const { pipe, abort } = renderToPipeableStream(jsx, { + onError: error => { + didError = true + console.error("error", error) + }, + onShellError: error => { + didError = true + console.log("shell error", error) + }, + onShellReady: () => { + res.statusCode = didError ? 500 : 200 + res.setHeader("Content-Type", "text/html; charset=utf-8") + pipe(stream) + }, + onAllReady: () => { + clearTimeout(streamTimeout) + }, + }) + + // Abandon and switch to client rendering if enough time passes. + streamTimeout = setTimeout(() => { + abort() + }, STREAM_TIMEOUT) + + return stream +} diff --git a/src/System/Router/__tests__/serverRouter.jest.enzyme.tsx b/src/System/Router/__tests__/serverRouter.jest.enzyme.tsx index d101f064840..b63c9a4dcae 100644 --- a/src/System/Router/__tests__/serverRouter.jest.enzyme.tsx +++ b/src/System/Router/__tests__/serverRouter.jest.enzyme.tsx @@ -38,6 +38,12 @@ jest.mock("@loadable/server", () => ({ jest.mock("react-tracking") +jest.mock("Server/config", () => { + return { + ENABLE_SSR_STREAMING: false, + } +}) + const defaultComponent = () =>
hi!
describe("serverRouter", () => { @@ -127,21 +133,21 @@ describe("serverRouter", () => { }) it("bootstraps relay SSR data", async () => { - const { scripts } = await getWrapper() - expect(scripts).toContain("__RELAY_BOOTSTRAP__") + const { extractScriptTags } = await getWrapper() + expect(extractScriptTags?.()).toContain("__RELAY_BOOTSTRAP__") }) it("does not prefix CDN_URL if not available", async () => { const postScripts = ` ` - const { scripts } = await getWrapper() - expect(scripts).toContain(postScripts) + const { extractScriptTags } = await getWrapper() + expect(extractScriptTags?.()).toContain(postScripts) }) it("prefixes CDN_URL to script tags if available", async () => { process.env.CDN_URL = CDN_URL const postScripts = ` ` - const { scripts } = await getWrapper() - expect(scripts).toContain(postScripts) + const { extractScriptTags } = await getWrapper() + expect(extractScriptTags?.()).toContain(postScripts) }) }) diff --git a/src/System/Router/renderServerApp.tsx b/src/System/Router/renderServerApp.tsx index b15d1ac2edf..621873412b6 100644 --- a/src/System/Router/renderServerApp.tsx +++ b/src/System/Router/renderServerApp.tsx @@ -3,6 +3,8 @@ import { getServerParam } from "Utils/getServerParam" import { renderToString } from "react-dom/server" import loadAssetManifest from "Server/manifest" import path from "path" +import { Transform } from "stream" +import { ENABLE_SSR_STREAMING } from "Server/config" import { getENV } from "Utils/getENV" import { ServerAppResults } from "System/Router/serverRouter" import { getWebpackEarlyHints } from "Server/getWebpackEarlyHints" @@ -26,6 +28,7 @@ interface RenderServerAppProps extends ServerAppResults { mount?: boolean req: ArtsyRequest res: ArtsyResponse + stream?: Transform } export const renderServerApp = ({ @@ -35,7 +38,8 @@ export const renderServerApp = ({ mount = true, req, res, - scripts, + extractScriptTags, + stream, styleTags, }: RenderServerAppProps) => { const headTagsString = renderToString(headTags as any) @@ -44,6 +48,8 @@ export const renderServerApp = ({ const { WEBFONT_URL } = sharify.data + const scripts = extractScriptTags?.() + const { linkPreloadTags } = getWebpackEarlyHints() const options = { @@ -66,7 +72,6 @@ export const renderServerApp = ({ fontUrl: WEBFONT_URL, imageCdnUrl: GEMINI_CLOUDFRONT_URL, icons: { - // TODO: Move to new assset pipeline, this adds the CDN for images. favicon: res.locals.asset("/images/favicon.ico"), faviconSVG: res.locals.asset("/images/favicon.svg"), appleTouchIcon: res.locals.asset("/images/apple-touch-icon.png"), @@ -76,11 +81,37 @@ export const renderServerApp = ({ openSearch: MANIFEST.lookup("/images/opensearch.xml"), webmanifest: MANIFEST.lookup("/images/manifest.webmanifest"), }, - // TODO: Post-release review that sharify is still used in the template. sd: sharify.data, } const statusCode = getENV("statusCode") ?? code - res.status(statusCode).render(`${PUBLIC_DIR}/html.ejs`, options) + res + .status(statusCode) + .render(`${PUBLIC_DIR}/html.ejs`, options, (error, html) => { + if (error) { + console.error(error) + + res + .status(500) + .send("Internal Server Error: Error rendering server app") + } else { + if (ENABLE_SSR_STREAMING && stream) { + res.write(html.split('
')[0]) + + // React mount point + res.write('
') + + // Start streaming HTML response + stream.pipe(res, { end: false }) + + stream.on("end", () => { + res.write("
") + res.end() + }) + } else { + res.send(html) + } + } + }) } diff --git a/src/System/Router/serverRouter.tsx b/src/System/Router/serverRouter.tsx index c798bb7ec03..35e4e66c047 100644 --- a/src/System/Router/serverRouter.tsx +++ b/src/System/Router/serverRouter.tsx @@ -30,7 +30,7 @@ export interface ServerAppResults { url: string } status?: number - scripts?: string + extractScriptTags?: () => string headTags?: any[] styleTags?: string } @@ -119,9 +119,10 @@ export const setupServerRouter = async ({ ) } - const { html, scripts, styleTags } = await collectAssets({ + const { html, stream, extractScriptTags, styleTags } = await collectAssets({ ServerRouter, relayEnvironment, + res, }) // Sentry names transactions according to their Express route. @@ -138,8 +139,9 @@ export const setupServerRouter = async ({ headTags, html, redirect, - scripts, + extractScriptTags, status, + stream, styleTags, } diff --git a/src/Typings/sharify.d.ts b/src/Typings/sharify.d.ts index e2c50254e3a..625a72d4db9 100644 --- a/src/Typings/sharify.d.ts +++ b/src/Typings/sharify.d.ts @@ -47,6 +47,7 @@ declare module "sharify" { ENABLE_CONVERSATIONS_MESSAGES_AUTO_REFRESH: boolean ENABLE_NEW_AUCTIONS_FILTER: boolean ENABLE_QUERY_BATCHING: boolean + ENABLE_SSR_STREAMING: boolean ENABLE_WEB_CRAWLING: string ENABLE_WEB_VITALS_LOGGING: boolean FACEBOOK_APP_NAMESPACE: string diff --git a/webpack/bundleSplitting.js b/webpack/bundleSplitting.js index 7a40902e0ad..1ee64e3c791 100644 --- a/webpack/bundleSplitting.js +++ b/webpack/bundleSplitting.js @@ -18,6 +18,14 @@ export const splitChunks = { cacheGroups: { default: false, defaultVendors: false, + // Contains the entrypoint for the client used for quick React rehydration + bootstrap: { + name: "bootstrap", + test: /src[\\/]client\.tsx$/, + chunks: "all", + enforce: true, + priority: 45, + }, "artsy-framework": { name: "artsy-framework", chunks: "all", @@ -79,6 +87,7 @@ export const splitChunks = { minChunks: TOTAL_PAGES, priority: 20, }, + shared: { name(module, chunks) { const cryptoName = crypto