0.9.0 (3) - add integration test with experimentally patched React version #219
Conversation
| "version": "0.0.0", | ||
| "type": "module", | ||
| "scripts": { | ||
| "prepare": "rm -rf node_modules/@apollo/client-react-streaming; mkdir node_modules/@apollo/client-react-streaming && cp -r ../../packages/client-react-streaming/{package.json,dist} node_modules/@apollo/client-react-streaming", |
There was a problem hiding this comment.
This is necessary so it doesn't pick up the react folder from packages/client-react-streaming/node_modules and mixes two react versions.
| import type { DataTransportProviderImplementation } from "@apollo/client-react-streaming"; | ||
| import { DataTransportContext } from "@apollo/client-react-streaming"; | ||
| import { useMemo, useActionChannel, useStaticValue, useRef } from "react"; | ||
|
|
There was a problem hiding this comment.
Note to self: double check the approach of queueing a single promise for request & response
| let vite; | ||
| let bootstrapModules = []; | ||
| let assets = []; | ||
| console.log({ isProduction }); |
There was a problem hiding this comment.
| console.log({ isProduction }); |
Not sure if this was meant to stay or not 🙂
| if (!isProduction) { | ||
| const { createServer } = await import("vite"); | ||
| vite = await createServer({ | ||
| server: { middlewareMode: true, hmr: true }, | ||
| appType: "custom", | ||
| base, | ||
| }); | ||
| app.use(vite.middlewares); | ||
| } else { | ||
| const compression = (await import("compression")).default; | ||
| const sirv = (await import("sirv")).default; | ||
| app.use(compression()); | ||
| app.use(base, sirv("./dist/client", { extensions: [] })); | ||
| const index = await readFile("./dist/client/index.html", "utf-8"); | ||
| for (const script of index.matchAll( | ||
| /<script type="module" \w+ src="(.*)">/g | ||
| )) { | ||
| bootstrapModules.push(script[1]); | ||
| } | ||
| for (const link of index.matchAll( | ||
| /<link rel="stylesheet" \w+ href="(.*)">/g | ||
| )) { | ||
| assets.push(link[1]); | ||
| } | ||
| } |
There was a problem hiding this comment.
I know this is meant to be more of a "demo" (ish), but any chance we could flip the conditional here? I always find it easier with if/else to use the positive connotation first.
Rather than:
if (!isProduction) {
// other envs
} else {
// production builds
}Use this:
if (isProduction) {
// production stuff
} else {
// other environments
}This keeps the "production" configuration closer to the conditional test for it, rather than breaking it apart.
There was a problem hiding this comment.
This is based on the Vite React SSR template and I'm trying to keep the diff as small as possible, so while I agree and would really like to swap it, it's probably better we don't :)
There was a problem hiding this comment.
I didn't realize these were copy-pasted so makes sense! Thanks for pointing me to that.
| res.send("<!doctype><p>Error</p>"); | ||
| }, | ||
| onError(x) { | ||
| console.log("Error", x); |
There was a problem hiding this comment.
| console.log("Error", x); |
Since you already have a console.error below, could we just use that?
There was a problem hiding this comment.
Uh yeah, I'm generally gonna remove a bunch of these logs :)
This adds an integration test for the "experimental React hooks" solution and moves the
@apollo/client-react-streaming/experimental-react-transportout of the package, only into the test.It's not overly complicated, and we probably don't need to really ship it as part of a package.