isker / neogrok

Neogrok is a frontend for zoekt, a fast and scalable code search engine.
https://neogrok-demo-web.fly.dev
MIT License
44 stars 3 forks source link

SSR #2

Closed isker closed 1 year ago

isker commented 1 year ago

Neogrok right now is only CSR'd. It's nice to have a statically servable application but any deployment of neogrok already needs a deployment of zoekt-webserver, so being CSR-only doesn't really make for a fundamentally simpler deployment situation.

The site is really only lacking SSR for expediency in getting it off the ground.

With a server, we could do things to improve performance even in CSR, such as wrapping the zoekt APIs to remove unneeded data from responses and getting the data we do need into a more readily renderable state, like all of the buffer/string slinging we do to render match chunks, and doing syntax highlighting, which will require large JS libraries to do with good fidelity; doing all of that on the server and sending a more straightforward response to the client ought to be a nice improvement to perf.

We should do SSR with a framework as rolling your own is quite perilous. Since we're already using react-router, the easiest framework to use would be Remix, though we'd need to drink react-router's "loader" kool-aid as a first step toward being migratable to Remix. The other prominent React option is Next. The wildcard option is SvelteKit, but that's certainly going to be a big lift.

I have experience with none of those frameworks, so it is a bit of an open question. The main constraint is finding one that supports the somewhat insane routing and history manipulation we do with live search.

isker commented 1 year ago

The main constraint is finding one that supports the somewhat insane routing and history manipulation we do with live search.

I looked into doing this with next.js (because React server components are The Future!) and while I could sort out the React problems in this regard (and in a much better way than the current React key approach; I will probably incorporate it into the existing CSR-only app), I could not make it work for nextjs-specific reasons. On popstate, next is incorrectly not going back to the server, ensuring that while the UI will update on back button, there will be no API call/rerender with the correct data.

This is tracked in issues like https://github.com/vercel/next.js/issues/42991 and https://github.com/vercel/next.js/issues/45026. It is a hard blocker until that's resolved. They've acknowledged the situation in the first issue but it's not clear when/where there will be a resolution.

I pushed the demo to https://github.com/isker/neogrok/tree/nextjs-demo.

I'll try remix next.

isker commented 1 year ago

Okay, so the next issues are fixed in 13.2.5-canary.9, but it's revealed the next problem: everything is unmounted on navigation, regardless of whether it's a server or client component. So we can't navigate on every keypress. The form remounts and loses focus, for example. Very janky. Maybe next just isn't an option for this kind of UI.

On the remix front, I have a WIP conversion to remix and it's going okay. The biggest annoyance is that remix unconditionally polyfills fetch and related web globals on the server, and their fetch polyfill is really bad. It does things like crash the server when fetches are aborted under certain circumstances that neogrok trivially triggered.

Even though modern nodes provide all the web APIs remix polyfills, you can't opt out of the polyfills; it always blows the native implementations away. This is in contrast to next's behavior, which uses the native implementations if available. If you strip the polyfills, the remix node runtime is actually very simple. I might reimplement it without the polyfills and try to start a conversation with the maintainers about it.

Because remix renders and hydrates the full document, it also has big problems with hydration mismatches on the latest released version of react when site users have any browser extensions that mutate dom. That is fixed in the next unreleased version of react, 18.3.0.

isker commented 1 year ago

I ended up making remix work using some gross yarn patches to overcome the polyfill problem. It's pushed to this branch: https://github.com/isker/neogrok/tree/remix.

That being said remix has some pretty brutal performance problems with the constant navigations that react-router did not, especially on the repositories page. I have not been able to get to the bottom of the problem, other than that it's related to the server-only load functions that are critical to my goal of minimizing code sent to the client, especially to do things like syntax highlighting. If you get rid of those loads, perf gets way better. Perf problems are much less noticable in prod builds but it's not exactly encouraging.

Overall, I am not very impressed with remix. A lot of debt incurred servicing the previous states of both React and node from a few years ago, and no indications that it's keeping up with more recent changes.

Since I've been learning so much about the landscape trying these conversions out, I am just going to go the distance and try out Svelte and SvelteKit.