-
Notifications
You must be signed in to change notification settings - Fork 10
[fix] Allow props to be passed down to child component #5
Conversation
CHANGELOG.md
Outdated
| # `react-markdown-github` | ||
|
|
||
| - Create prop pass-through to provide props to `react-markdown` | ||
| - Change `resolver` prop to `transformLinkUri` to be consistent with `react-markdown` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please notate as **BREAKING**
dusave
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Once you've updated the CHANGELOG, 👍
indexzero
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's avoid adding new dependencies like rip-out if we can do this in pure JSX.
src/index.js
Outdated
| 'renderers', | ||
| 'transformImageUri', | ||
| 'transformLinkUri' | ||
| ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you do the spread first in JSX this shouldn't be necessary, right? e.g.
render() {
const renderers = {
heading: this.renderHeading,
...this.props.renderers
};
return <ReactMarkdown { ...this.props }
renderers={ renderers }
transformLinkUri={ this.transformLinkUri }
transformImageUri={ this.transformImageUri } />;
}There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually, that would work now. Originally, I suggested that to be able to override the transformLinkUri and transformImageUri calls, but that’s no longer needed since they’re called as callbacks in those functions. Let’s do it that way, as long as we don’t change it in the future to not allow the user to provide their own override of a prop.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed. @QuicheSama please update this to use the JSX spread technique outlined in my previous comment.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Addressed in 46b0158
react-markdown-githubdepends onreact-markdownbut does not pass mostreact-markdownprops down to it's dependent component. This prevents users from overriding properties to get the behavior, or access the features they need. This simply passes the props through.