[chore] cleanup Router and Renderer
#4101
Conversation
|
|
Is this intended as an alternative to combining them into a single |
benmccann
changed the title
renderer field from RouterRouter and Renderer
benmccann
force-pushed
the
router-refactor-2
branch
from
5f7187c to
70bceab
Compare
| addEventListener('touchstart', trigger_prefetch); | ||
| addEventListener('mousemove', handle_mousemove); | ||
| addEventListener('sveltekit:trigger_prefetch', trigger_prefetch); | ||
| addEventListener('hashchange', () => this._update_page_store(new URL(window.location.href))); |
There was a problem hiding this comment.
Inside this event listener needs to be that if (hash_navigating) that gets inited as false, and mutated to true in click event listener if the navigation only changes hash, and also inside it needs to have that history.replaceState.
There was a problem hiding this comment.
Actually I just checked and you have duplicate for every event listener, so this is probably WIP still, or?
There was a problem hiding this comment.
Ummm. I'm pretty sure there is not a duplicate for every event listener unless I'm missing something. There are two hashchange listeners though, which I hadn't noticed. Perhaps they should be combined so that the handlers execute in a consistent order
There was a problem hiding this comment.
Yeah, I saw those 2 and assumed that there are more duplicates, sorry for that, but if that's the case then I guess we can merge them, if code should be after that other one for consistency.
|
Closing in favor of #4161 after discussion with the other maintainers |
I know there's been some talk of combining
RouterandRendererinto a single class, but I think the original idea of having separateRouterandRendererimplementation came from a good place and we just never implemented it very wellThe benefits of having a separate
RouterandRendererinclude being able to hydrate without loading theRoutercodeand more possibilities for improved
Routertesting.This removes
rendereras a field fromRouter. By removing cycles between the two classes, it's far easier to understand what's happening and what should happen where. One of the main things needed to accomplish this is to move prefetching to theRenderer. It makes more sense inRendererthanRoutersince it almost renders the page by callingload, etc. whereas theRouteris meant to be just related to navigationThings this PR cleans up the API quite a bit. The set of public methods on the two classes is much saner now.