Assign props at the top level of render #326
Conversation
|
This pull request is automatically deployed with Now. |
johno
force-pushed
the
props-fix
branch
2 times, most recently
from
2d3b050 to
d554d76
Compare
ChristopherBiscardi
left a comment
ChristopherBiscardi
left a comment
There was a problem hiding this comment.
Just spent a bit following the commits over the last day or two that added/reverted the components changes, etc. This seems like it'll work as long as we pass everything into the createElement call. The other option I can think of would've been adding a conditional destructuring off this.props in the case of having an export.
| constructor(props) { | ||
| super(props) | ||
| this.layout = ${layout} | ||
| this.layout = ${layout || 'null'} |
There was a problem hiding this comment.
could also remove the entire constructor instead if layout isn't defined.
There was a problem hiding this comment.
I think this is semantically cleaner than omitting the constructor entirely, because it's communicating that there is no user layout. null is semantically different than undefined.
| } | ||
| render() { | ||
| const { components = {} } = this.props | ||
| const { components = {}, ...props } = this.props |
There was a problem hiding this comment.
won't this need an equivalent change in the runtime to pass the props into createElement?
Previously, the runtime was just using the scoped names when passed in to defined the components and props variables. If we're standardizing the class to be this.props access always, then we need to pass everything into createElement in the runtime like
createElement(MDXContent, {components, ...props})
|
I can't push it up to this branch, but this additional change passes the tests for me. diff --git a/packages/runtime/src/index.js b/packages/runtime/src/index.js
index 12f2bdd..0cd13de 100644
--- a/packages/runtime/src/index.js
+++ b/packages/runtime/src/index.js
@@ -37,7 +37,7 @@ export default ({
...keys,
`${code}
- return React.createElement(MDXContent, { components });`
+ return React.createElement(MDXContent, { components, ...props });`
)
return fn({}, React, ...values) |
|
Thanks @ChristopherBiscardi, that diff was the ticket for runtime 👍 |
No description provided.