Skip to content

refactor(router-core): make parsePathnameCache internal#4767

Closed
dsonet wants to merge 2 commits intoTanStack:mainfrom
dsonet:router-core-cache-parse-path-refactor
Closed

refactor(router-core): make parsePathnameCache internal#4767
dsonet wants to merge 2 commits intoTanStack:mainfrom
dsonet:router-core-cache-parse-path-refactor

Conversation

@dsonet
Copy link

@dsonet dsonet commented Jul 25, 2025

REF: #4752

It introduced the LRU ache for parsePathname. However, it exposed the internal cache, making it the caller's responsibility to pass the cache. This could defeat the purpose if the parameter is missing.

I'd suggest it's better to keep the cache internal and transparent to the caller. This reduces the burden of passing the cache around and keeps the API cleaner, the bundle size would be smaller as well.

@dsonet dsonet changed the title refactor(router-core): make LRU cache internal refactor(router-core): make parsePathnameCache internal Jul 25, 2025
@nx-cloud
Copy link

nx-cloud bot commented Jul 25, 2025

View your CI Pipeline Execution ↗ for commit ad42b7b

Command Status Duration Result
nx affected --targets=test:eslint,test:unit,tes... ✅ Succeeded 12s View ↗

☁️ Nx Cloud last updated this comment at 2025-07-25 11:23:18 UTC

@Sheraff
Copy link
Contributor

Sheraff commented Jul 25, 2025

We intentionally made it a thing that lives on the router, because depending on the setup, some people have several routers. The cache is passed to the appropriate methods internally, and no public APIs were affected. What is your use-case that is impacted by this change?

@schiller-manuel
Copy link
Contributor

we specifically chose to make the cache router instance specific. all the functions parsePathname etc. are not public API.
we'll leave the implementation as is.
in future, can you please create an issue so we can discuss before you spend the effort to create a PR?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants