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

Jump to bottom

Tracking Issue: Full-page client-side navigation #60951

Closed
4 of 14 tasks
DAreRodz opened this issue Apr 22, 2024 · 9 comments
Closed
4 of 14 tasks

Tracking Issue: Full-page client-side navigation #60951

DAreRodz opened this issue Apr 22, 2024 · 9 comments
Labels
[Feature] Interactivity API API to add frontend interactivity to blocks. [Packages] Interactivity /packages/interactivity [Type] Tracking Issue Tactical breakdown of efforts across the codebase and/or tied to Overview issues.

Comments

@DAreRodz
Copy link
Contributor

DAreRodz commented Apr 22, 2024 •
edited by michalczaplinski
Loading

What problem does this address?

Full-page, client-side navigation (for lack of a better name) is another navigation mode we want to include in the Interactivity API along with the current region-based, client-side navigation already implemented.

Region-based navigation works by updating the HTML of those regions defined on the current and target pages. When there are no matching regions between the two pages, navigation is delegated to the browser, and the page is reloaded. In such cases, the application's JS state is reset, and it is not possible to implement features such as keeping a video open or a song playing while navigating the page.

In contrast, full-page navigation updates the HTML of the entire page when navigating without the need to specify regions. This allows you to transition from one page to another on the site while maintaining the application's state. This is, in fact, what is implemented at https://wpmovies.dev/, although that implementation needs to be polished and improved before it can be published.

In that regard, @SantosGuillamot has created a PR with a first iteration under an experimental flag in #59707, although work remains to be done, especially in relation to the handling of new assets appearing on the target page, whether inside <head> or <body>. This feature is also missing for region-based navigation.

Current tasks

The following list of tasks is based on feedback received in #59707:
@DAreRodz DAreRodz added [Type] Tracking Issue Tactical breakdown of efforts across the codebase and/or tied to Overview issues. [Feature] Interactivity API API to add frontend interactivity to blocks. [Packages] Interactivity /packages/interactivity labels Apr 22, 2024
@DAreRodz DAreRodz mentioned this issue Apr 22, 2024
Merged
@westonruter westonruter mentioned this issue Apr 29, 2024
Merged
@westonruter
Copy link
Member

Explore how to modify the body tag without the hack: link.

See #61212

@juanmaguitar juanmaguitar mentioned this issue May 6, 2024
Merged
@luisherranz luisherranz changed the title Full-page client-side navigation tracking issue Tracking Issue: Full-page client-side navigation Jul 8, 2024
@luisherranz luisherranz mentioned this issue Jul 8, 2024
Closed
30 tasks
@michalczaplinski
Copy link
Contributor

Add e2e tests: #59707 (comment).

I've started implementing some new e2e tests for full-page client-side navigation in #63268

@michalczaplinski
Copy link
Contributor

I'd like to understand a bit better what remains to be done to improve the handling of assets:

  • Change innerText for textContent and revisit how styles are injected: link.

This should be a trivial 1 line change.

I was the person who reported the original issue, and this issue is still present. We might have to implement something like the suggested fix.

  • If the stylesheet remains the same, it should probably be left as-is: link. It would be nice to be able to reuse DOM diffing from Preact here if possible.

We can't use Preact to render the <head> but we should look into this (#59707 (comment)) to prevent performance issues.

  • Ensure caching is required: [link]

This should be a trivial change.

Additionally, we should only load the module scripts belonging to the new blocks on the page or other modules that have declared their compatibility with full-page client-side navigation (link).

Are you aware of any other considerations for asset loading? cc @luisherranz @DAreRodz @cbravobernal

@michalczaplinski
Copy link
Contributor

michalczaplinski commented Jul 8, 2024 •
edited
Loading

I looked at https://github.com/hotwired/turbo to see how they handle assets, and it seems like they kinda side-step the problem by not prefetching the scripts and styles 😄.

They merge the "old" <head> with the new one in a way similar to us:

https://github.com/hotwired/turbo/blob/c339144924e5632d4ef5e3a7b941b3db17d9efa3/src/core/drive/page_renderer.js#L72-L83

Interestingly, they never fetch() the scripts and styles.

Hotwired can prefetch an HTML page for an <a> link (like Interactivity API). However, it waits for the page navigation to happen and, upon navigation, injects the new styles & scripts into the head and lets the browser figure out how to load them.

EDIT: This is effectively the same strategy for loading assets that @gziolo and I suggested here & here

@DAreRodz
Copy link
Contributor Author

DAreRodz commented Jul 9, 2024

Nice! Maybe this is useful as well: a while ago, I did a quick experiment, using the htmx head-support extension as an inspiration. 😁

It also injects HTML tags and uses onload and onerror event handlers to keep track of which assets have been loaded so far, but instead of adding the assets directly to the HTML, it appends <link> elements with rel=preload, so the new styles or scripts are not displayed/executed until effectively inserted into the HTML.

The htmx head-support extension also has other interesting features, like special attributes to control the merge behavior or script re-evaluation.

@SantosGuillamot SantosGuillamot mentioned this issue Jul 25, 2024
Closed
@luisherranz
Copy link
Member

Pasting this @michalczaplinski's comment from this PR to have the conversation centralized 🙂

We should investigate if calling the fetch() ourselves to prefetch & cache scripts is the right way to go as investigated by @DAreRodz in this branch: 9baff87. At the minimum, we should consider passing { priority: low } in the fetch requests.

A few things:

  • We can't use onload on the rel=preload/rel= modulepreload links because by the spec, they are not guaranteed to be prefetched. We need to put the onload on the real links or use fetch and inject the content ourselves.

  • For scripts, as we will only support modules, we can use link rel="modulepreload" for preloading, and dynamic imports for loading. We only need to await on the dynamic imports once we want to actually show the new page.

    Something like this:

    await Promise.all(newModules.map((module) => import(module)));

  • I'm not quite sure about the CSS strategy yet. We'll probably be fine using link rel="preload" for preloading and await on the final link tags onload events once we want to actually show the new page (apart from copying over the inlined style tags).

@luisherranz luisherranz mentioned this issue Aug 6, 2024
Merged
@michalczaplinski
Copy link
Contributor

Thanks Luis!

We need to put the onload on the real links or use fetch and inject the content ourselves.

yup, that was my idea too 🙂 But as we're going to use modules then, for scripts, we only have to do the second thing that you mention which is this:

we can use link rel="modulepreload" for preloading, and dynamic imports for loading. We only need to await on the dynamic imports once we want to actually show the new page.

For CSS, I was considering something like what you suggested, but I'm also not yet 100% sure. I'll probably have to try it out in a prototype.

@ockham ockham mentioned this issue Sep 3, 2024
Merged
@michalczaplinski
Copy link
Contributor

Status Update

#64067 has been merged, which fixes #63880. It improves the handling of JS assets when doing full-page client-side navigation and brings the following improvements:

  • Remove the src attributes from prefetched script tags.
  • Use .textContent instead of .innerText to set <script> contents.
  • Wait for the load event of the script element before evaluating it.
  • Only prefetch module scripts; never prefetch regular scripts. That's
    because regular scripts (without async or defer attributes) found in the
    head are blocking and must be executed in order. When prefetching there is no
    guarantee that the scripts will execute in the order they are prefetched.
    Module scripts can be executed in any order.

Now, follow-up PRs should focus on:

  1. Handle CSS asset loading in a way analogous to how JS assets are handled now:
    1. Handle properly the cache busting hash in the URLs of the block styles. (comment)
    2. Moving the styles from a file to <style> tags messes up with how the URLs are interpreted in CSS's url() function. (comment)
    3. CSS should probably be pre-loaded using <link> tags as discussed in this comment. It could possiblly leverage this approach:
    4. Fix other issues with styling: Interactivity API: Styling is broken after navigation when using experimental full-page client-side navigation #66123
  2. Figure out the way to only load the scripts for the new & compatible interactive blocks on the page. Previously mentioned in this comment & this comment
  3. Handle fetching more robustly, consider using Promise.allSettled() (comment)
  4. (low priority) Refactor the fetchHeadAssets() function so that it's not called inside of regionsToVdom.

@luisherranz
Copy link
Member

luisherranz commented Nov 25, 2024 •
edited
Loading

Superseded by this, as this is going to be the focus for WP 6.8.

Let's move the conversation there 🙂

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
[Feature] Interactivity API API to add frontend interactivity to blocks. [Packages] Interactivity /packages/interactivity [Type] Tracking Issue Tactical breakdown of efforts across the codebase and/or tied to Overview issues.
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@westonruter @luisherranz @michalczaplinski @DAreRodz