search

nextstrain / nextstrain.org

The Nextstrain website
https://nextstrain.org
GNU Affero General Public License v3.0
87 stars 49 forks source link

Replace Next.js Links with HTML anchor #885

Closed victorlin closed 3 weeks ago

victorlin commented 4 weeks ago

Description of proposed changes

Consistently use server-side navigation links across the codebase.

Checklist

tsibley commented 4 weeks ago

What @genehack said... I think I'd prefer to either decide now to use <Link> or <a> (and in general I'd prefer the latter) rather than do a refactor which avoids making that decision.

genehack commented 4 weeks ago

(and in general I'd prefer the latter)

FWIW I lean in that direction too, but I noticed from the next/link docs that <Link> does some opportunistic pre-fetching, and I wondered how removing that might affect perceived site responsiveness.

tsibley commented 4 weeks ago

Yeah, it's not just pre-fetching, it's also the difference between client-side navigation and round trips to the server.

victorlin commented 4 weeks ago

I don't have a strong opinion here, but is this part of the broader discussion at https://github.com/nextstrain/private/issues/88?

tsibley commented 4 weeks ago

Sorta, yeah. "Do we want to have client-side navigation between pages? Or is it overkill for nextstrain.org?"

I personally see client-side nav for page-oriented sites like nextstrain.org as effectively an optimization layer, and find that it's often not worth the added complexity/quirks it brings (even when that complexity is wrapped up and tied with a bow by a framework like Next.js).

victorlin commented 4 weeks ago

This should really be discussed on an issue which I should've created before PR, but I think we've enough conversation to continue here rather than starting fresh with a new issue. I also don't want to close this PR or block on https://github.com/nextstrain/private/issues/88 because there's no good reason for the current mix of client and server side navigation.

Given the votes here, I'll rework this PR to replace all client side navigation with server side navigation and drop the wrapper.