fix(nextjs): Use separate buffers for each awaitable navigation type #3480
Conversation
🦋 Changeset detectedLatest commit: 553cf16 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
|
||
| if (windowNav) { | ||
| window.__clerk_internal_navFun = (to, opts) => { | ||
| registerNavigationType(name).fun = (to, opts) => { |
There was a problem hiding this comment.
| registerNavigationType(name).fun = (to, opts) => { | |
| const registerNavigation = registerNavigationType(name) | |
| registerNavigation.fun = (to, opts) => { |
❓ Is there any issue changing to the above, so we don't always running the function as it is called many times?
| registerNavigationType(name).promisesBuffer = []; | ||
| } | ||
| window.__clerk_internal_navPromisesBuffer.push(res); | ||
| registerNavigationType(name).promisesBuffer!.push(res); |
There was a problem hiding this comment.
❓ Does this need to be always push, shouldn't we calling push/replace depending the strategy? (Not exactly part of what the PR changes here but I'm curious)
There was a problem hiding this comment.
This is separate from the concept of router push/replace, we're just pushing a promise onto the internal buffer here. I think this should be okay.
| registerNavigationType(name).promisesBuffer = []; | ||
| } | ||
| window.__clerk_internal_navPromisesBuffer.push(res); | ||
| registerNavigationType(name).promisesBuffer!.push(res); |
There was a problem hiding this comment.
This is separate from the concept of router push/replace, we're just pushing a promise onto the internal buffer here. I think this should be okay.
| const registerNavigationType = (name: string) => { | ||
| if (!window.__clerk_internal_navigations) { | ||
| window.__clerk_internal_navigations = {}; | ||
| } | ||
|
|
||
| if (!(name in window.__clerk_internal_navigations)) { | ||
| // @ts-ignore | ||
| window.__clerk_internal_navigations[name] = {}; | ||
| } | ||
|
|
||
| return window.__clerk_internal_navigations[name]; | ||
| }; |
There was a problem hiding this comment.
nit, this feels more like a getter:
| const registerNavigationType = (name: string) => { | |
| if (!window.__clerk_internal_navigations) { | |
| window.__clerk_internal_navigations = {}; | |
| } | |
| if (!(name in window.__clerk_internal_navigations)) { | |
| // @ts-ignore | |
| window.__clerk_internal_navigations[name] = {}; | |
| } | |
| return window.__clerk_internal_navigations[name]; | |
| }; | |
| const getClerkNavigationObject = (name: string) => { | |
| window.__clerk_internal_navigations ??= {}; | |
| window.__clerk_internal_navigations[name] ??= {} | |
| return window.__clerk_internal_navigations[name]; | |
| }; |
| // as ClerkProvider might be unmounted and remounted during navigations | ||
| // If we use a ref, it will be reset when ClerkProvider is unmounted | ||
| window.__clerk_internal_navPromisesBuffer = []; | ||
| registerNavigationType(name).promisesBuffer = []; |
There was a problem hiding this comment.
Could be simplified with:
| registerNavigationType(name).promisesBuffer = []; | |
| registerNavigationType(name).promisesBuffer ??= []; |
Can remove the wrapped if that way.
…-integration-test
panteliselef
deleted the
elef/sdk-1782-debug-failing-navigationtestts-integration-test
branch
Description
Checklist
npm testruns as expected.npm run buildruns as expected.Type of change