Skip to content
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

Ssr doesn't work if use getContext in entries.ts 's defineRouter/getComponent #104

Closed
EasonYou opened this issue Jul 11, 2023 · 5 comments · Fixed by #124
Closed

Ssr doesn't work if use getContext in entries.ts 's defineRouter/getComponent #104

EasonYou opened this issue Jul 11, 2023 · 5 comments · Fixed by #124

Comments

@EasonYou
Copy link
Contributor

What's the issue

I use ssr with routes and getContext

As the code show, it doesn't work if use getContext in defineRouter/getComponent.

https://github.com/EasonYou/waku-example/blob/71308df89396c7fa6b5c9ce54ff46d8fafcdf7aa/src/entries.ts#L1-L21

image

What do I find

The error tells that ctx was empty, it cause by the getComponent so that waku can't get element by getSsrConfig because of the component import error. Then the ssr will not work. But no error on the client side, just like a csr app.

waku/src/router/server.ts

Lines 155 to 171 in 5af1c34

const getSsrConfig: GetSsrConfig = async (pathStr) => {
const url = new URL(pathStr, "http://localhost");
// We need to keep the logic in sync with waku/router/client
// FIXME We should probably create a common function
const pathItems = url.pathname.split("/").filter(Boolean);
const rscId = pathItems.join("/") || "index";
try {
await getComponent(rscId);
} catch (e) {
// FIXME Is there a better way to check if the path exists?
return null;
}
return { element: [SSR_PREFIX + url.pathname, { search: url.search }] };
};
return { getEntry, getBuildConfig, getSsrConfig };
}

I don't know if it is a purposeful design that user can't use getSsrConfig or it is a bug.

@dai-shi
Copy link
Owner

dai-shi commented Jul 12, 2023

Thanks for reporting!
Does it fix or change something if you comment out L162 in src/router/server.ts?

@EasonYou
Copy link
Contributor Author

No, if you commont out L162, all the ssrConfig will be got in the ssr middleware(see the code below L99). It cause that all the request wiil be handled as a ssr page.
I have time to try to fix it, maybe I can create a pr later.

if (req.url && !req.headers["x-waku-ssr-mode"]) {
const ssrConfig = getSsrConfig && (await getSsrConfig(req.url));
if (ssrConfig) {
const rscServer = new URL(
config.framework.ssr.rscServer,
"http://" + req.headers.host
);

@dai-shi
Copy link
Owner

dai-shi commented Jul 12, 2023

Oh, yeah, that's right..

I think we should avoid using getComponent in getSsrConfig.

@EasonYou
Copy link
Contributor Author

Index component is default as the routes' entry, but I find that if I visit index path, it will render the index components as child.
image

@dai-shi
Copy link
Owner

dai-shi commented Jul 12, 2023

Oh, I wasn't aware that could happen. (So, it's a bug...)

Right, I think how we handle index is a bit weird compared to community standard.
I would like to fix/improve the file name convention and support [slug].tsx too.

Let's tackle them separately.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants