feat(core): Add route provider API for parameterized route resolution - #23551
feat(core): Add route provider API for parameterized route resolution#23551logaretm wants to merge 1 commit into
Conversation
size-limit report 📦
|
905570f to
488ffe0
Compare
|
I requested a draft review because I need opinions on this stack but may not be around to merge this till next week. |
Lms24
left a comment
Lms24
left a comment
There was a problem hiding this comment.
I thought a bit about the router provider and I like the idea of it! Initially, I was wondering if we could just get away with a url -> route map or cache. But the idea of having a "live" resolver for the framework SDKs that support resolving a url to a route is a good one! Bonus points that this is not something users have to configure but is (for now) baked into browserTracingIntegrations. We can see where exactly this makes more sense as we split up the integration into sub integrations.
I suggest running this by the framework WG since it will primarily concern framework SDKs. I looked into a couple of the stacked framework PRs and the changes look fairly minimal which is nice!
Just had some minor nits for now
| * Normalizes to a real `URL` so providers never have to parse, and relative locations (which memory | ||
| * routers hand around) resolve against the document. | ||
| */ | ||
| function toURLObject(url: string): URL | undefined { |
There was a problem hiding this comment.
l: I think we already have a helper like this in core 🤔
There was a problem hiding this comment.
l: As long as we're planning on adding the provider only to browser-side SDKs, can we move this to browser-utils? Francesco is working on moving all browser-specific exports over, so I think we can take the shortcut and add it to utils right away.
Framework SDKs can register a provider that resolves a URL to a low-cardinality route name, so integrations stop each reaching for the route their own way. Wires up `bfcacheIntegration` as the first consumer: its segment name ends up as a metric dimension, where an unparameterized URL is unbounded cardinality.
488ffe0 to
3754c50
Compare
That was one of my goals, breaking up the dependency between route parameterization and tracing because parameterization could be needed in logs/metrics/errors or whatever. This brings me to the next point, so it feels like we need a breaking change to break up this dependency in those frameworks, for example the Vue SDK accepts the router in The base API is fine, but I will need to do some more work on the stack in downstream SDKs.
Will post in the channel! |
|
This was a 9 PR stack. It's now only a few, I wanted to record the reasoning. Every provider ended up registered inside a tracing integration, which is the coupling the work was meant to break. The deletions I expected only unlock once
So these 2 PRs do the smallest useful thing, parameterization that doesn't need tracing and that only fits in Next.js and Remix because these are only ones that have decoupled implementations (by virtue of the routes being accessible globally), Astro may also fit as well. I think what I have here is a good base that we can either merge today and improve later, or expand on the base API to address these concerns. |
2 participants
Adds the route provider API to core so framework SDKs can register how to resolve a URL to a parameterized route, and wires up
bfcacheMetricsIntegrationas the first consumer since that is where the fragmentation currently produces a raw URL on a metric dimension.bfcacheMetricskeeps its pathname fallback, so nothing changes until a provider is registered.closes #23556