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(client): avoid mismatching between route path and page data (close #1249) #1361

Merged
merged 3 commits into from
Jul 11, 2023

Conversation

Mister-Hope
Copy link
Member

@Mister-Hope Mister-Hope commented Jun 10, 2023

Use sync afterEach to update route and pageData in a sync process.

@meteorlxy
Copy link
Member

@Mister-Hope If I understand correctly, if we update page data in beforeResolve, there might be a short mismatch between route path and page data, because we are using an async callback.

So you move the assignment to afterEach with a sync callback, right?

What if users add another afterEach async callback themselves? Maybe the mismatch would still happen?

@Mister-Hope
Copy link
Member Author

Mister-Hope commented Jul 1, 2023

I think that hook will be after this one as this one is the first hook, and don't affect the page data update time.

@Mister-Hope
Copy link
Member Author

What if users add another afterEach async callback themselves? Maybe the mismatch would still happen?

The router does not care afterEach hooks, it just call them directly with forEach (regradless they return promise or not).

I tested locally and it does not mismatch. @meteorlxy Maybe apply this fix with vite 4.4 and a new version?

@meteorlxy meteorlxy changed the title fix(client): fix route.path changed after pageData, close #1249 fix(client): avoid mismatching between route path and page data (close #1249) Jul 11, 2023
@meteorlxy meteorlxy merged commit 9b0ad9e into main Jul 11, 2023
29 checks passed
@meteorlxy meteorlxy deleted the route-path branch July 11, 2023 12:03
meteorlxy added a commit that referenced this pull request Jul 11, 2023
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.

None yet

2 participants