Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Expose HAST nodes to components #19

Closed
Rokt33r opened this issue Apr 10, 2020 · 14 comments · Fixed by #22
Closed

Expose HAST nodes to components #19

Rokt33r opened this issue Apr 10, 2020 · 14 comments · Fixed by #22
Labels
🗄 area/interface This affects the public interface 💪 phase/solved Post is done 🧒 semver/minor This is backwards-compatible change 🦋 type/enhancement This is great to have

Comments

@Rokt33r
Copy link
Contributor

Rokt33r commented Apr 10, 2020

Subject of the feature

I want to access HAST nodes from React components to use position or custom attributes of nodes when rendering.

Problem

For now, there is no way to access because rehype-react is tightly bound to hast-to-hyperscript, which consumes Node.

Expected behavior

A React component should be able to access its Node info somehow.

Alternatives

Passing a node instance via node prop.

@Rokt33r Rokt33r added 🙉 open/needs-info This needs some more info 🦋 type/enhancement This is great to have labels Apr 10, 2020
@Rokt33r Rokt33r changed the title Expose node to components Expose HAST nodes to components Apr 10, 2020
@wooorm
Copy link
Member

wooorm commented Apr 10, 2020

What would this look like? Do you get the whole subtree of the node that is to be compiled? Can you access children of a node too (which are compiled before the parent?)

The simplest solution I can think of to begin solving it is:

https://github.com/syntax-tree/hast-to-hyperscript/blob/6337edb309209ba2ec8c2431daa779f50d2a7ad2/index.js#L141:

  // Ensure no React warnings are triggered for void elements having children
  // passed in.
  result =
-   elements.length === 0 ? h(name, attributes) : h(name, attributes, elements)
+   elements.length === 0 ? h.call(node, name, attributes) : h.call(node, name, attributes, elements)

  // Restore parent schema.
  ctx.schema = parentSchema

...as passing it in props will probably cause things to fail because some tools would serialize node as an attribute?

...then we have access to it here:

rehype-react/index.js

Lines 39 to 43 in 761597e

// Wrap `createElement` to pass components in.
function h(name, props, children) {
var component = has.call(components, name) ? components[name] : name
return createElement(component, props, children)
}

...and can choose to put it in a prop for components?

@Rokt33r
Copy link
Contributor Author

Rokt33r commented Apr 10, 2020

What would this look like? Do you get the whole subtree of the node that is to be compiled? Can you access children of a node too (which are compiled before the parent?)

For now, I just need to access the node being compiled. But I guess some users might want to access whole subtree too.

The simplest solution I can think of to begin solving it is: ...

The solution looks good to me. Is there any way to modify the h function from rehype-react? If it doesn't exist, I guess I should fork this package and implement customized h function.

@wooorm
Copy link
Member

wooorm commented Apr 11, 2020

The first example shows how h.call is added to hast-to-hyperscript, which exposes the node that is transformed in rehype-react’s h as this.

We could then change rehype-react to something like this:

 function h(name, props, children) { 
   var component = name
   if (has.call(components, name)) {
     component = components[name]
     props.node = this
   }
   return createElement(component, props, children) 
 } 

@Rokt33r
Copy link
Contributor Author

Rokt33r commented Apr 11, 2020

LGTM. And I think we should make it optional too for users who are using prop-types package. So I guess I should submit a pr to hast-to-hyperscript first?

@wooorm
Copy link
Member

wooorm commented Apr 11, 2020

PR to h2h is a good first step, I don’t see a harm in changing that.

I do feel like an option to pass node to every component is a bit weird. 🤔 How would this (whether to pass a prop or not) normally go in React-land?

@Rokt33r
Copy link
Contributor Author

Rokt33r commented Apr 11, 2020

I do feel like an option to pass node to every component is a bit weird

How is it weird?

@wooorm
Copy link
Member

wooorm commented Apr 12, 2020

I mean, a single option passNode or so that passes props.node to every component, is that powerful enough? Or will people then want an option passNodeTo: ['ThisComponent', 'ThatComponent']? Will people also want to access parent in props potentially?

@Rokt33r
Copy link
Contributor Author

Rokt33r commented Apr 12, 2020

I mean, a single option passNode or so that passes props.node to every component, is that powerful enough?

I think this sounds enough in my opinion. passNodeTo: ['ThisComponent', 'ThatComponent'] feels too much.

Will people also want to access parent in props potentially?

I think this can be done while transforming. So I don't think we need it.

@wooorm
Copy link
Member

wooorm commented Apr 12, 2020

Alright, sounds like we have a plan! Are you interested / able to work on it?

@Rokt33r
Copy link
Contributor Author

Rokt33r commented Apr 12, 2020

Yeah, I'll do it tonight or tomorrow!

wooorm pushed a commit to syntax-tree/hast-to-hyperscript that referenced this issue Apr 24, 2020
@wooorm
Copy link
Member

wooorm commented May 19, 2020

So this is 50% there. Last part is to update here, right?

@wooorm
Copy link
Member

wooorm commented Jun 25, 2020

@Rokt33r No rush at all, but a friendly ping!

@Rokt33r
Copy link
Contributor Author

Rokt33r commented Jul 21, 2020

Ah sorry. I'll do it now..! I was too busy...

wooorm pushed a commit that referenced this issue Jul 24, 2020
Related to: syntax-tree/hast-to-hyperscript#23.
Closes GH-19.
Closes GH-22.

Reviewed-by: Christian Murphy <[email protected]>
Reviewed-by: Titus Wormer <[email protected]>
@wooorm wooorm added ⛵️ status/released 🗄 area/interface This affects the public interface 🦋 type/enhancement This is great to have 🧒 semver/minor This is backwards-compatible change and removed 🙉 open/needs-info This needs some more info 🦋 type/enhancement This is great to have labels Jul 24, 2020
@wooorm
Copy link
Member

wooorm commented Jul 24, 2020

Released in 6.1.0!

@wooorm wooorm added the 💪 phase/solved Post is done label Aug 7, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🗄 area/interface This affects the public interface 💪 phase/solved Post is done 🧒 semver/minor This is backwards-compatible change 🦋 type/enhancement This is great to have
Development

Successfully merging a pull request may close this issue.

2 participants