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

Fix subrouter createURL #154

Merged
merged 10 commits into from
Nov 9, 2023
Merged

Fix subrouter createURL #154

merged 10 commits into from
Nov 9, 2023

Conversation

willybrauner
Copy link
Member

@willybrauner willybrauner commented Nov 8, 2023

When with createUrl of a sub-router, from a sub-router, the URL wasn't build properly on the server-side. This cause a diff of render between server and client.

To resolve this issue we need to:

  • Get an "is root router" state on server and client side
  • Harly reset all Routers property on each http request (because the nodejs runtime is not clear on each http request)
  • Rework createUrl() function: internalize all the getUrlByRouteName logic
  • Update and split tests
  • Remove old core getUrlByRouteName() & getFullPathByPath()
  • Remove core compileUrl(), use path-to-regexp compile instead

Copy link

codesandbox-ci bot commented Nov 8, 2023

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.

Latest deployment of this branch, based on commit 8f22538:

Sandbox Source
example-client Configuration
example-ssr Configuration

@willybrauner
Copy link
Member Author

willybrauner commented Nov 8, 2023

size-limit report 📦

Path Size
@cher-ami/router 8.08 KB (-0.97% 🔽)

@willybrauner willybrauner force-pushed the fix-subrouter-createurl branch 3 times, most recently from 1caab5c to 25c647e Compare November 9, 2023 11:29
@willybrauner willybrauner merged commit 69f840a into main Nov 9, 2023
3 checks passed
@willybrauner willybrauner deleted the fix-subrouter-createurl branch November 9, 2023 14:40
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 this pull request may close these issues.

1 participant