fix: server router #501
fix: server router #501
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 1 Ignored Deployment
|
|
This pull request is automatically built and testable in CodeSandbox. To see build info of the built libraries, click here or the icon next to each commit SHA. |
| @@ -239,13 +240,17 @@ export const renderHtml = async ( | |||
| default: { createFromReadableStream }, | |||
| }, | |||
| { ServerRoot }, | |||
| { ServerRouter }, | |||
There was a problem hiding this comment.
This is not what we want. waku/router should work without changing the core. (otherwise, other router integration like TanStack Router wouldn't be possible.)
You should change getSsrConfig in unstable_defineRouter. Let me know if you need any help.
packages/waku/src/router/client.ts
Outdated
| if (!isServer) { | ||
| throw new Error('ServerRouter is not supposed to be run in the client'); | ||
| } |
There was a problem hiding this comment.
We don't need this check (at least for now). But, maybe we need some comments telling it's not a public API, also for ServerRoot.
|
Sure, I'll resolve the comments soon! |
Which ones? |
Aslemammad
force-pushed
the
fix/server-router
branch
from
f31201e
to
8600e2a
Compare
Aslemammad
force-pushed
the
fix/server-router
branch
from
692beb3
to
6ac0b33
Compare
packages/waku/src/router/client.ts
Outdated
| export function ServerRouter({ | ||
| children, | ||
| loc, | ||
| }: PropsWithChildren<{ loc: ReturnType<typeof parseLocation> }>) { |
There was a problem hiding this comment.
Just my preference:
| }: PropsWithChildren<{ loc: ReturnType<typeof parseLocation> }>) { | |
| }: { children: ReactNode; loc: ReturnType<typeof parseLocation> }) { |
There was a problem hiding this comment.
I'll revert this since it would force us to pass children and children would force us to have keys if we pass array, unless we pass a fragment, which does not look good too.
There was a problem hiding this comment.
It's just a TS issue, right?
I can take care of it later.
There was a problem hiding this comment.
hmm, ts issue, key issue + code formatting issue
Aslemammad
force-pushed
the
fix/server-router
branch
from
84d98ee
to
7274344
Compare
Co-authored-by: Daishi Kato <dai-shi@users.noreply.github.com>
Aslemammad
force-pushed
the
fix/server-router
branch
from
ab4fe0e
to
57bffa5
Compare
| "react": "18.3.0-canary-14fd9630e-20240213", | ||
| "react-dom": "18.3.0-canary-14fd9630e-20240213", | ||
| "react-server-dom-webpack": "18.3.0-canary-14fd9630e-20240213", |
|
Yep
…On Wed, 21 Feb 2024, 05:21 Daishi Kato, ***@***.***> wrote:
***@***.**** approved this pull request.
LGTM. Nice work!
------------------------------
In package.json
<#501 (comment)>:
> - "react": "18.3.0-canary-14fd9630e-20240213",
- "react-dom": "18.3.0-canary-14fd9630e-20240213",
- "react-server-dom-webpack": "18.3.0-canary-14fd9630e-20240213",
Was this causing the CI issue?
—
Reply to this email directly, view it on GitHub
<#501 (review)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AJBMICDX6SID7WJNPLCPJVDYUVHLJAVCNFSM6AAAAABDOJAGTWVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMYTQOJRHA4TMNJZGY>
.
You are receiving this because you authored the thread.Message ID:
***@***.***>
|
Resolves #417
This PR adds ServerRouter and injects it for the routes in the server, so users don't hit the "Missing router error"
on Discord: