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.

Sign up for GitHub

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

[Experiment] Full-page navigations using Speculation Rules #60959

Draft
wants to merge 5 commits into
base: trunk
Choose a base branch
from
Draft

[Experiment] Full-page navigations using Speculation Rules #60959

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
5955d08
Dirty test that shares state between current page and prerendered one
DAreRodz Apr 13, 2024
4a81546
changes
cbravobernal Apr 13, 2024
a9c53ab
Some other changes
DAreRodz Apr 13, 2024
1d12f75
Merge branch 'trunk' into experiment/interactivity-api-speculation-ru…
michalczaplinski Apr 22, 2024
e773038
persist in sessionStorage
michalczaplinski Apr 22, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
persist in sessionStorage
  • Loading branch information
@michalczaplinski
michalczaplinski committed Apr 22, 2024
commit e773038ba9290c30567ffe8dc892daef1c517b30
20 changes: 14 additions & 6 deletions packages/interactivity/src/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,13 @@ document.addEventListener( 'DOMContentLoaded', async () => {
await init();
} );

function storeStore() {
window.sessionStorage.setItem(
'interactivity-api-store',
serializeStore()
);
}

if (
// @ts-ignore
document.prerendering
Expand All @@ -68,12 +75,13 @@ if (
'interactivity-api-store'
);
populateInitialData( JSON.parse( store ) );

// remove the store from session storage
window.sessionStorage.removeItem( 'interactivity-api-store' );

// add the listener for when the user leaves the page
window.addEventListener( 'beforeunload', storeStore );
Comment on lines +82 to +83
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As opposed to only updating the store when the user unloads the page, what if it is consistently updated throughout the life of the page? Imagine then if you had two windows open side-by-side. As you make changes in one window, you could then see them update in the other window. Naturally this would involve adding a storage event and not just a prerenderingchange. And performance would need to be protected against, given that sessionStorage is a synchronous API. Also, this could be annoying to a user as it could very well be that they want the state to differ between two windows (e.g. comparing flights for two different vacation options).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So nevermind 😄

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right :)

I'm becoming more convinced that we might want to offer a separate "MPA mode" and "SPA mode" for full-page navigations.

It might even be possible to mix and match the two modes on the same site. So you can imagine that navigating around product pages in an e-commerce site uses SPA mode, but then navigating to the "about us" or "shipping rates" pages uses the MPA mode.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the client-side state is preserved in MPA mode (e.g. using some approach as in this PR), would there then need to be an SPA mode?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It might be tough to reliably preserve the client-side state in MPA mode. The current PR does it for the state from the store but we would need to do the same for context and getElement(). (link).

Perhaps there might be some way to expose the context in the Interactivity API and let the framework merge it upon navigation. That would need quite a bit of work though 😄 .

That said, choosing between SPA and MPA for navigation in an application is an architectural decision. So, we can't slap SPA navigation on a site that assumes it was built for MPA or vice versa. Because of this, even if preserving all client-side state (state, context & element) can be done, we would still need the user to decide between SPA mode and MPA mode.

} );
} else {
window.addEventListener( 'beforeunload', () => {
window.sessionStorage.setItem(
'interactivity-api-store',
serializeStore()
);
} );
window.addEventListener( 'beforeunload', storeStore );
}
5 changes: 1 addition & 4 deletions packages/interactivity/src/store.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -368,10 +368,7 @@ const deepClone = ( source: any ) => {
export const serializeStore = () => {
const stateData = { state: {} };
for ( const [ namespace, value ] of stores.entries() ) {
stateData.state[ namespace ] = JSON.stringify(
deepClone( value.state )
);
stateData.state[ namespace ] = deepClone( value.state );
}

return JSON.stringify( stateData );
};
Loading