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

⚛️ Add directives to handle providesContext and usesContext#68

Merged
DAreRodz merged 5 commits intomain-full-vdom-hydrationfrom
full-vdom/block-context
Sep 15, 2022
Merged

⚛️ Add directives to handle providesContext and usesContext#68
DAreRodz merged 5 commits intomain-full-vdom-hydrationfrom
full-vdom/block-context

Conversation

@DAreRodz
Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
Member

Choose a reason for hiding this comment

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

Copy link
Copy Markdown
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
Copy Markdown
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