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: fallback frame when navigating #948

Merged
merged 4 commits into from
Mar 8, 2022
Merged

fix: fallback frame when navigating #948

merged 4 commits into from
Mar 8, 2022

Conversation

rigor789
Copy link
Member

@rigor789 rigor789 commented Mar 2, 2022

This handles a case where a frame with a different id might have overridden the previous frame with the same id, but has been unmounted.

/cc @farfromrefug

@farfromrefug
Copy link
Contributor

i am not sure i understand. if no options.frame is passed and there is no more frame with id default in n-vue frame cache (because there are actually 2 frames with id default and one was removed) then there is no fallback to Frame.topmost()?

@rigor789
Copy link
Member Author

rigor789 commented Mar 2, 2022

getFrame(id) returns a frame from the frame Map - which is set/unset in the <Frame> component mounted/destroyed callbacks.

So when we destroy the 2nd default frame it's unset from the Map - but then $navigateTo is called, and by default the frame option is set to default, so it ends up using Frame.getFrameById('default') which returns the remaining Frame<default> that we pass as a fallback to getFrame(id, fallback) -> which then adds it back to the frame Map and returns it's vue VM instance... Essentially restoring it's entry to the frame Map.

It's a bit weird how we handle this, and the reason we even have the frame Map is because core will only return the frame from getFrameById if it has been navigated at least once... Perhaps it would make sense to patch that in core at some point and then refactor this whole setup to not rely on the frame Map.

@farfromrefug
Copy link
Contributor

@rigor789 thanks for the explanation. I get it now! seems good to me

@rigor789 rigor789 merged commit 6bad6c8 into master Mar 8, 2022
@rigor789 rigor789 deleted the fix/frame-fallback branch March 8, 2022 14:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants