Skip to content
This repository has been archived by the owner on Jul 28, 2023. It is now read-only.

Hydrate views even if they are registered after the DOM nodes appear #57

Merged

Conversation

luisherranz
Copy link
Member

@luisherranz luisherranz commented Aug 24, 2022

@DAreRodz and I did a pair programming this morning that resulted in this PR to fix #52.

What

We changed the logic to make sure there are no race conditions between the View components being registered and the custom elements' connectedCallback execution.

Why

Because sometimes the registration will happen after the connectedCallback execution.

How

Create a global registry that can be accessed by any party, and delay the hydration of the custom elements that have no View component registered yet.

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


Additional comments

I've also renamed registerBlockType to registerBlockView as an alternative to #56 and removed all references to the word "frontend", even in file names.

cc: @c4rl0sbr4v0: would you mind taking a look at that? Thanks!

Copy link
Collaborator

@cbravobernal cbravobernal left a comment

Choose a reason for hiding this comment

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

Looks good to me! A lot of code refactor 😄

Copy link
Collaborator

@DAreRodz DAreRodz left a comment

Choose a reason for hiding this comment

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

Nice implementation and refactoring, Luis. 👏

Comment on lines +13 to +15
['wp', 'view', name].reduce((obj, name, i) => {
if (typeof obj[name] === 'undefined')
obj[name] = i === 2 ? initialValue : {};
Copy link
Collaborator

Choose a reason for hiding this comment

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

Interesting way to initialize a nested property inside an undefined object. 🙂

I would use the length from the array instead of hardcoding it to check if we have reached the property with the initial value:

Suggested change
['wp', 'view', name].reduce((obj, name, i) => {
if (typeof obj[name] === 'undefined')
obj[name] = i === 2 ? initialValue : {};
['wp', 'view', name].reduce((obj, name, i, array) => {
if (typeof obj[name] === 'undefined')
obj[name] = i === (array.length - 1) ? initialValue : {};

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, but why calculate array.length - 1 when it is always 2 anyway. And if someone changes the array length in the future and doesn't update the hardcoded value, this thing won't work at all. So I think it's fine with the hardcoded 2 😄🤷‍♂️

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm going to merge this as it is, but feel free to refute my argument!

@luisherranz luisherranz merged commit 75f5865 into main Aug 26, 2022
@luisherranz luisherranz deleted the register-block-type-and-connected-callback-execution-order branch August 26, 2022 10:21
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
3 participants