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

⚛️ Add directives to handle providesContext and usesContext #68

Merged
merged 5 commits into from
Sep 15, 2022

Conversation

DAreRodz
Copy link
Collaborator

Closes #67

I've refactored toVdom significantly and added support for directives like in WordPress/gutenberg#44034. There are still some code I haven't moved to directives, like sourced attributes generation and inner blocks location (not sure if it is possible).

Also, I created three directives:

  • One that handles type and renders the block component (if it exists).
  • Another one that handles providesContext and renders a wrapper around block children.
  • And the last one, which handles usesContext, that injects a context property to a block component using a HOC.

Directives are placed in src/directives.

Comment on lines +58 to +59
// Create vNode. Note that all `wpBlock` props should exist now to make directives work.
const vNode = h(type, props, children);
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Here I've assumed that options.vnode would run every time h() is used. Maybe I'm wrong here. 🤔

Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Member

@luisherranz luisherranz left a comment

Choose a reason for hiding this comment

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

At first glance, it looks great to me. I'm glad the current implementation of the directives is flexible enough to inject atomic functionality into the vdom. However, I was hoping we could delegate more to the wrapper component and less to the vdom internals.

But we can keep playing with this API in other PRs.

@DAreRodz
Copy link
Collaborator Author

However, I was hoping we could delegate more to the wrapper component and less to the vdom internals.

I left three things inside toVdom internals:

  1. Sourced attributes computation ― it needs access to the original DOM, at least with the current implementation.
  2. Block props processing (class to className, style string to object, etc.) ― I guess we can move that inside the wrapper component without much problem.
  3. Inner blocks location ― this can also be moved to the wrapper component, although I found it convenient to do that during toVdom() execution. We can give this a thought in the future.

Thanks for your review @luisherranz. I'm merging this PR. 😄

@DAreRodz DAreRodz merged commit 1131b54 into main-full-vdom-hydration Sep 15, 2022
@DAreRodz DAreRodz deleted the full-vdom/block-context branch September 15, 2022 10:46
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants