search

WordPress / block-interactivity-experiments

⚠️ WARNING: This repository is no longer active. The development and discussions have been moved to the Gutenberg repository.
https://github.com/WordPress/gutenberg/discussions/categories/interactivity-api
109 stars 11 forks source link

Tracking issue: Custom Elements Hydration 🧩 #39

Closed luisherranz closed 1 year ago

luisherranz commented 2 years ago

This issue aims to list and keep track of the tasks related to the Custom Elements Hydration experiment.

For the Directives Hydration experiment, please check the Tracking Issue: Directives Hydration ⚛️.

The main branch of this experiment is main-custom-elements-hydration. Make sure you select it when opening a pull request if it is related to this experiment.

Please proactively comment on this issue whenever you start working on a new task, get blocked, or finish a task, sharing as much detail and information as possible. Thanks!!

Done

Required to expose as experimental API in Gutenberg

Future improvements

luisherranz commented 2 years ago

Previous video summaries

Originally posted in https://github.com/WordPress/block-hydration-experiments/issues/6

Update 1

https://www.loom.com/share/549258dfe6844fe0b3fdc8c3fc856b13

For context, when I mention Turbolinks in the video, I'm referring to this type of client-side navigation: https://github.com/WordPress/gutenberg/pull/38713

Update 2

https://www.loom.com/share/e5a5541d027d4149a91a70f30b0913be

Resources mentioned:

cbravobernal commented 2 years ago

I'm currently working on #38. Still digging and trying to discover the best way to duplicate the Context Provider and sync the value of that context.

michalczaplinski commented 2 years ago

I have started working on adding support for providing context from non-interactive blocks.

cbravobernal commented 2 years ago

Added a WIP draft PR for #38 -> #40

michalczaplinski commented 2 years ago

Resharing this brief status update on my work on #28:

I was able to make it work so far in the simplest case: one static (non-interactive) parent and one child interactive block 👍 .

Most immediate next steps would be:

michalczaplinski commented 2 years ago

I've finished the work on Support providing context from non-interactive blocks #28. It's gonna go through some review now but should be close to what we want to merge 🙂 .

cbravobernal commented 2 years ago

The React Context API subtasks are 90% complete! We still need to check how it works on the editor, where it should be easy.

I will update the tracking issue again as soon as we unsubscribe the contexts after using them.

luisherranz commented 2 years ago

I've opened an issue to see how we could unmount the interactive-block components before they are removed from the DOM:

luisherranz commented 2 years ago

I've also added Matías' nomenclature suggestions to the list, although I'd wait until the rest of the PRs are merged to avoid merge conflicts.

cbravobernal commented 2 years ago

The last commits are adding a double Map to link blocks to context, and then the values and update values functions to those block->Context-> Value

On the other hand, I will next take a look if we can suppress custom warnings for the useEffect, or either way, look for an alternative to run the code once without it.

Weakmap seems to be valid in most edge cases, but @luisherranz offered to take a deeper view to see if it fits our requirements.

I will then wait for #28 to be merged, so we can check later if there are any conflicts and we can start testing altogether.

michalczaplinski commented 2 years ago

Luis has suggested that we try a slightly different approach in that PR in order to avoid using a custom element to wrap all blocks that pass context.

The idea is to serialize the context and pass it as an HTML attribute of the top-level HTML element of the block itself (as opposed to setting it on the custom element). Then, we can use a MutationObserver on the document to detect changes to the DOM and pass the context whenever that happens. This is necessary because the "new", updated DOM might contain new interactive blocks that are subscribed to some piece of context.

My worry is that this can have poor performance because we'll have to fire the MutationObserver callback on every DOM change and the DOM changes can come from unexpected sources (browser extensions, ads, third party scripts, etc.)

Next step: I will make a quick test to try to see how bad the new implementation would perform if there are very frequent DOM updates.

cbravobernal commented 2 years ago

-https://github.com/WordPress/block-hydration-experiments/pull/40 status update:

I added another Context to see if it works and seems to be failing. Feel free to keep working on that, as I will be 2 weeks AFK!

luisherranz commented 2 years ago

Before we start adding code to the WooCommerce Block Hydration Experiments repo, I'd like to finish the three PRs we have open right now:

And once those are merged, I'd like to do the nomenclature refactor:

Not important for Woo, but at that point, I'd also like to remove dprint, and test prettier with the prettier-php plugin.

luisherranz commented 2 years ago

I've finished the support for the React Context API (https://github.com/WordPress/block-hydration-experiments/pull/40), including a code refactoring of frontend.js, which was getting a bit out of hand (https://github.com/WordPress/block-hydration-experiments/pull/40/commits/d61391cf240e82b5edafbd8f3af081f41331318d). I've also resolved the conflicts on https://github.com/WordPress/block-hydration-experiments/pull/28 and I'm working to resolve them in https://github.com/WordPress/block-hydration-experiments/pull/41.

luisherranz commented 2 years ago

I've switched to prettier in https://github.com/WordPress/block-hydration-experiments/pull/43 and resolved conflicts on https://github.com/WordPress/block-hydration-experiments/pull/28 and https://github.com/WordPress/block-hydration-experiments/pull/41.

@michalczaplinski, @ockham everything should be fine now.

Once you finish, I'll do the nomenclature changes.

luisherranz commented 2 years ago

https://github.com/WordPress/block-hydration-experiments/pull/28 should be ready now. I've made a video to explain the changes because I unified everything in gutenberg-block.

I want to take a fresh look at the inner blocks template to see if we can avoid this problem now (and don't wait if/until we do this in Gutenberg):

EDIT: BTW, I also added prettier-php in https://github.com/WordPress/block-hydration-experiments/pull/28 to test it out.

luisherranz commented 2 years ago

@ockham finished https://github.com/WordPress/block-hydration-experiments/pull/41 🎉🎉

I'm going to resolve the conflicts with https://github.com/WordPress/block-hydration-experiments/pull/28.

luisherranz commented 2 years ago

I'm going to resolve the conflicts with #28.

Done. I think everything should be ready now.

michalczaplinski commented 2 years ago

As Luis mentioned in this comment already, the approach of using the MutationObserver did not work so we're still going to wrap the non-interactive blocks with a custom element. However, Luis has moved the serialization to PHP so we're not saving the context in the content in save() anymore.

luisherranz commented 2 years ago

Thanks, Michal 🙂

I'll work on https://github.com/WordPress/block-hydration-experiments/issues/37 today, and if I can, I'll check if https://github.com/WordPress/block-hydration-experiments/issues/29 can be solved during render_block.

luisherranz commented 2 years ago

I've opened an issue to talk about prettier-php. If you're playing with this repository, please help us test it and if you have any problem, please report it here:

luisherranz commented 2 years ago

The PR for the nomenclature changes is ready for review:

luisherranz commented 2 years ago

Update 3

Ok, now that we've kind of reached another milestone, I did another video summary to explain the latest changes.

https://www.loom.com/share/3b68a95ac80f43ecbe86ca8225cb35a0

luisherranz commented 2 years ago

I've opened a new issue for the bug I discovered during the video:

michalczaplinski commented 2 years ago

The PR for the nomenclature changes is ready for review:

I've merged this PR as well! 🙂

SantosGuillamot commented 2 years ago

I've created this repo to replicate the experiments done here but using Alpinejs. The main idea is to be able to compare the developer experience and limitations of both approaches. Right now, I just recreated the current blocks, but we will probably run a few more tests there.

DAreRodz commented 2 years ago

I took the liberty of merging the fix for the WpBlock unmounting issue (https://github.com/WordPress/block-hydration-experiments/issues/42) as it was a simple PR and was tested already. 😄

I also updated the tracking issue description with the latest changes.

luisherranz commented 2 years ago

I've updated the Up Next list with two new items:

ockham commented 2 years ago

I've filed #51 (which Carlos approved and merged -- thanks!) to fix

luisherranz commented 2 years ago

I've opened the issue to make sure the order of execution of the registerBlockType() and connectedCallback is not important:

luisherranz commented 2 years ago

@darerodz and I have explored a fix for the race condition problem (https://github.com/WordPress/block-hydration-experiments/issues/52):

Reviews are welcomed!

DAreRodz commented 2 years ago

I've discussed with @luisherranz what things we need to solve before exposing these APIs as experimental from Gutenberg.

It ended in a to-do list, included in the issue description. ☝️

luisherranz commented 2 years ago

I've added a bit more context to the Export different code depending on the context issue.

DAreRodz commented 2 years ago

I created a new issue for the remaining item in the "Required for experimental API" list.

luisherranz commented 2 years ago

I've added a proposal for an alternative wrapperless hydration method based on a static virtual DOM of the full page:

luisherranz commented 2 years ago

We opened a new issue to discuss possible solutions to locate the children of the islands/client-components (moved from #60):

luisherranz commented 2 years ago

@darerodz and I have created a new Tracking Issue to track the work related to the Directives Hydration:

From now on, please use this Tracking Issue for the Custom Elements Hydration experiment and the other one for the Directives Hydration experiment.

We've also renamed the main branch to main-custom-elements-hydration for this experiment and created a new one called main-directives-hydration for the other one. Please, select the correct branch when opening pull requests.

SantosGuillamot commented 2 years ago

As explained in this issue, it looks like the setTimeout callback we are using inside connectedCallback is not 100% reliable. So we may need to look for an alternative.

luisherranz commented 2 years ago

To avoid confusion, I've renamed "Island Hydration" to "Custom Elements Hydration" because both this and Directives are just ways of doing island hydration.

luisherranz commented 1 year ago

Closed as we're not actively working on this experiment anymore. Our work continues in:

fabiankaegy commented 1 year ago

Closed as we're not actively working on this experiment anymore. Our work continues in:

@luisherranz I've been trying to track down the reasons why this approach was abandoned :) Would you be able to shed some light on that? :) Thanks in advance!

luisherranz commented 1 year ago

It had a few issues:

fabiankaegy commented 1 year ago

Thanks for sharing that insight :)