Skip to content
This repository has been archived by the owner on Feb 23, 2024. It is now read-only.

Interactivity API: make current filter blocks (client-side rendered) work with client-side navigation #8456

Closed
DAreRodz opened this issue Feb 16, 2023 · 18 comments
Assignees
Labels
status: stale Stale issues and PRs have had no updates for 60 days.

Comments

@DAreRodz
Copy link
Collaborator

DAreRodz commented Feb 16, 2023

We must ensure that current filter blocks can use navigate from the Block Interactivity API to update the Products block content.

The filters we need to modify are these:

They all call a function named changeUrl() in utils/filters.ts.

/**
* Change the URL and reload the page if filtering for PHP templates.
*
* @param {string} newUrl New URL to be set.
*/
export function changeUrl( newUrl: string ) {
if ( filteringForPhpTemplate ) {
window.location.href = newUrl;
} else {
window.history.replaceState( {}, '', newUrl );
}
}

By modifying that function in a way that it calls the navigate() function from the Interactivity API when client-side navigations are enabled, we would be making all blocks update the Products block content without having to reload the whole page.

The challenge here will be to make these block import navigate from a different bundle (the Interactivity API's runtime bundle).

@DAreRodz DAreRodz changed the title Make the current filters (CSR) work with client-side navigation. We are not doing the SSR yet. Interactivity API: make current filter blocks (client-side rendered) work with client-side navigation Feb 16, 2023
@DAreRodz DAreRodz self-assigned this Feb 16, 2023
@DAreRodz
Copy link
Collaborator Author

The challenge here will be to make these block import navigate from a different bundle (the Interactivity API's runtime bundle).

@luisherranz, do you know how to consume a bundle as a module from a different bundle (if that's possible in Webpack)? 🤔

@luisherranz
Copy link
Collaborator

So @woocommerce/interactivity is not treated as an external dependency?

@luisherranz
Copy link
Collaborator

I mean, are you trying to import it using something similar to this?

import { navigate } from "@woocommerce/interactivity";

@DAreRodz
Copy link
Collaborator Author

So @woocommerce/interactivity is not treated as an external dependency?

I mean, are you trying to import it using something similar to this?

import { navigate } from "@woocommerce/interactivity";

That's exactly what I'd like to do. 😅

However, by doing so, each block contains its own copy of the Interactivity API runtime―even using React instead of Preact, because the bundling process is different for these blocks.

@DAreRodz
Copy link
Collaborator Author

DAreRodz commented Feb 22, 2023

BTW, I did a quick test assigning navigate to a global variable in window, and everything seems to work fine. ✨

EDIT: not 100% right, there are cases where the filters disappear. It happens occasionally with multiple filters at the same time

@luisherranz
Copy link
Collaborator

However, by doing so, each block contains its own copy of the Interactivity API runtime―even using React instead of Preact, because the bundling process is different for these blocks.

So it seems like something is not working as expected, right?

We should ping someone from Woo to take a look at why an import to @woocommerce/interactivity is adding it to the bundle instead of enqueuing it as a dependency.

@luisherranz
Copy link
Collaborator

there are cases where the filters disappear. It happens occasionally with multiple filters at the same time

Do you think that problem is related to the bundling or it's something different?

@DAreRodz
Copy link
Collaborator Author

Do you think that problem is related to the bundling or it's something different?

It should be something different. I'm assigning the global navigate from our bundle, enqueuing it, and calling window.wpx.navigate from the changeUrl function mentioned above.

We should ping someone from Woo to take a look at why an import to @woocommerce/interactivity is adding it to the bundle instead of enqueuing it as a dependency.

Maybe @gigitux or @Aljullu could help us here. 😊

@luisherranz
Copy link
Collaborator

there are cases where the filters disappear. It happens occasionally with multiple filters at the same time

Well... we probably would have to add wp-ignore to those nodes to make Preact ignore their client-side rendered modifications of the DOM.

But don't spend time on this because we won't use full-page client-side navigation in Woo and that problem should disappear once we refactor the router to work only on the inner HTML of the Products block.

Maybe @gigitux or @Aljullu could help us here.

Luigi, Albert: if you don't know what may be the problem, please ping someone else from Woo. Thanks 🙂

@luisherranz
Copy link
Collaborator

If the problem is very complex to solve, we can use the global variable solution. But we should name it something like window.__experimentalWooStore (or whatever naming Woo uses for their experimental APIs) to avoid problems with extenders.

@gigitux
Copy link
Contributor

gigitux commented Feb 23, 2023

I'm not sure I understand the issue, but it seems there is an issue with the webpack configuration.
I'm not very expert on this, maybe @senadir or @thealexandrelara have some ideas about fixing the issue!

@Aljullu
Copy link
Contributor

Aljullu commented Feb 23, 2023

I'm not an expert either, but one idea would be to try using wcDepMap and wcHandleMap. That's what we use for externals which are shared across blocks, if I'm not wrong.

I see @woocommerce/interactivity is currently listed as an alias, but that's not exactly the same, an alias doesn't mean it will be unique across blocks. Instead, an external will guarantee it's only loaded once for all blocks. If you go this way, you will also need to add it as a core entry for it to be built.

@DAreRodz
Copy link
Collaborator Author

[...] one idea would be to try using wcDepMap and wcHandleMap.

Instead, an external will guarantee it's only loaded once for all blocks. If you go this way, you will also need to add it as a core entry for it to be built.

Thanks @Aljullu, I'll explore that approach. 🙏

@senadir
Copy link
Member

senadir commented Feb 23, 2023

Exactly what Albert said, in order to have a singleton you need to either assign the instance to some global variable (in window) or move the script to its own output build file that gets enqueued.

If it's not a large file I think doing window global is better? while also making sure you're not overriding it or trying to initiate several different instances.

@DAreRodz
Copy link
Collaborator Author

Thank you, @senadir. I did a quick test following @Aljullu's approach and it seems to be working fine.

I'm going to open a PR and start with the development. 🙂

@github-actions
Copy link
Contributor

This issue has been marked as stale because it has not seen any activity within the past 60 days. Our team uses this tool to help surface issues for review. If you are the author of the issue there's no need to comment as it will be looked at.

@github-actions github-actions bot added the status: stale Stale issues and PRs have had no updates for 60 days. label Apr 27, 2023
@roykho
Copy link
Member

roykho commented Sep 1, 2023

Hi @DAreRodz

Just realized this task was created awhile ago. We're currently working on this as well in our Epic woocommerce/woocommerce#42255

So checking with you if you're still working on this?

@DAreRodz
Copy link
Collaborator Author

DAreRodz commented Sep 5, 2023

So checking with you if you're still working on this?

Hey @roykho, thanks for the ping. I'm not working on this anymore (in fact, it's been a lot since I set this issue "on hold" in the Tracking Issue). I'll close this, as you're already working on your implementation for the Filter blocks.

@DAreRodz DAreRodz closed this as completed Sep 5, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
status: stale Stale issues and PRs have had no updates for 60 days.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants