-
Notifications
You must be signed in to change notification settings - Fork 2.2k
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
feat(react): add SSR support to React apps #13234
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
2b2d84f
to
5618614
Compare
5618614
to
f0b1e6f
Compare
f0b1e6f
to
5479f2c
Compare
5479f2c
to
f2689b1
Compare
f2689b1
to
b5934e6
Compare
9572e58
to
ec1a900
Compare
ec1a900
to
260b3ea
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! 🎉
I left a couple of non-blocking comments/questions.
"setup-ssr": { | ||
"factory": "./src/generators/setup-ssr/setup-ssr#setupSsrSchematic", | ||
"schema": "./src/generators/setup-ssr/schema.json", | ||
"description": "Set up SSR configuration for a project.", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I know it's a little verbose, but can we mention react project
to prevent any ambiguity?
}, | ||
}, | ||
}, | ||
serve: { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It may not be a big deal, but in Angular we maintain serve
as pointing to the browser build, and have a serve-ssr
for running browser + server. We don't have a single serve-server
to just serve the server.
So we align these? I followed the nguniversal approach of having serve
and serve-ssr
along with a server
target with the last being the actual build for the server.
I don't mind, it can be cleanup, but not sure if we want the same DX across both or not
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see. I think it's not useful to have serve
only do CSR. Especially if the server provides endpoints needed by the client (like backend-for-frontend).
@@ -48,31 +48,3 @@ function getNextAsyncIteratorFactory(options) { | |||
} | |||
}; | |||
} | |||
|
|||
export async function* mapAsyncIterator<T = any, I = any, O = any>( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Moved to its own file.
}, | ||
}, | ||
}, | ||
serve: { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see. I think it's not useful to have serve
only do CSR. Especially if the server provides endpoints needed by the client (like backend-for-frontend).
260b3ea
to
9bf3354
Compare
9bf3354
to
f084749
Compare
This pull request has already been merged/closed. If you experience issues related to these changes, please open a new issue referencing this pull request. |
Add a generator
@nrwl/react:setup-ssr
that generates SSR support and minimal Express server for serving the SSR and CSR app.Changes
@nrwl/react:setup-ssr
generator that adds an SSR entry point (main.server.ts
) for React apps, and aserver.ts
file that contains a simple Express app.@nrwl/web:ssr-dev-server
executor that wraps the client and server build targets as a single target. Allows options override viabrowserTargetOptions
andserverTargetOptions
.@nrwl/js
so we can reused them in thessr-dev-server
(we can consider moving them to devkit as well)Example
Say there is a React app
demo
atapps/demo
, then user can runnx g @nrwl/react:setup-ssr demo
, and the following three files will be added: