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

Firefox & Safari loses store navigating because they does NOT support Navigation API yet #688

Open
aralroca opened this issue Dec 17, 2024 · 6 comments
Assignees
Labels
awaiting external dep bug Something isn't working needs investigation wontfix This will not be worked on

Comments

@aralroca
Copy link
Collaborator

aralroca commented Dec 17, 2024

Firefox & Safari loses store navigating because they does NOT support Navigation API yet.

Maybe we need a temporal alternative like use the sessionStorage for the store to then recover? 🤔 We need to investigate it.

CC: @AlbertSabate

@aralroca aralroca added bug Something isn't working needs investigation labels Dec 17, 2024
@aralroca aralroca self-assigned this Dec 17, 2024
@aralroca aralroca added the high label Dec 17, 2024
@aralroca
Copy link
Collaborator Author

aralroca commented Dec 18, 2024

@AlbertSabate maybe as a workaround we can use something like this:

const storage = sessionStorage.getItem('s');
if(storage) $window._S = [...($window._S ?? []), ...JSON.parse(storage)]
$window.addEventListener('beforeunload', (event) => sessionStorage.setItem('s', JSON.stringify(Array.from($window._s.Map.entries()))))

here:

for (const [key, value] of $window._S ?? []) {
globalStore.Map.set(key, value);
}

@AlbertSabate
Copy link
Collaborator

Sounds like a nice workaround to have. For me there are couple of things to consider:

  • Consistency cross browser, they all have to act on the same way. Thinking that this should be implemented on all browsers but for the ones using Navigation API, only if native is enabled? This will be consistent cross browser while maintaining the nice improvements of Navigation API.
  • What happens on reload if I've updated the store, will also get this triggered if user reloads the page? Else it could have miss match of information.

@aralroca
Copy link
Collaborator Author

Probably we can use some flag like keepStoreToStorage (name to define), and we can support: sessionStoragelocalStorage | none (none by default)

@aralroca
Copy link
Collaborator Author

aralroca commented Dec 18, 2024

However, this approach only addresses the issue of the store being consumed directly by the client. However, it does not resolve these following scenarios:

  • Client → Server action modifies the store + redirect → soft-redirect from the client → new page request with the store → server renders the page based on the store
  • Client modifies the store → new page request with the store → server renders the page based on the store

It only resolves these cases:

  • Client → Server action modifies the store + redirect → soft-redirect from the client → new page request → web components consume the store directly
  • Client modifies the store → new page request → web components consume the store directly

The limitation is that the store is not stored in a cookie or any other mechanism accessible from the server. This means the server cannot retrieve the updated store when rendering the new page, leaving the state disconnected between client and server.

@aralroca
Copy link
Collaborator Author

aralroca commented Dec 18, 2024

This task remains pending for the time being. I lower its priority. Before we do anything messy like involving storage, cookies, or params to fix this, let's wait for Safari issue and Firefox issue to support the Navigation API. We will only take up this issue in case we see that it is very demanded by the Brisa community.

For now, there is this polyfill for the Navigation API https://github.com/virtualstate/navigation (bun i @virtualstate/navigation)

@aralroca aralroca added awaiting external dep wontfix This will not be worked on and removed high labels Dec 18, 2024
@aralroca
Copy link
Collaborator Author

aralroca commented Dec 18, 2024

as a recommendation: to share a state between pages, use the parameters when navigating. It also makes no sense for a page to be different when navigating than when rendering from scratch. When Safari and Firefox support the Navigation API, you will be able to use the store between pages as it works right now with Chrome / Edge.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
awaiting external dep bug Something isn't working needs investigation wontfix This will not be worked on
Projects
None yet
Development

No branches or pull requests

2 participants