-
Notifications
You must be signed in to change notification settings - Fork 332
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
fix(render): pass renderer.render
to default render
function
#940
Conversation
This pull request is automatically built and testable in CodeSandbox. To see build info of the built libraries, click here or the icon next to each commit SHA. Latest deployment of this branch, based on commit 7c2f351:
|
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.
👍
render({ children }, root) { | ||
render(children as ReactElement, root); | ||
}, | ||
renderer: { createElement, Fragment, render: render as Render }, |
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.
why is the cast needed? is the type too narrow to accept react's render?
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.
Yes, but it's assignable to Render
so the cast is valid.
const defaultRender: AutocompleteRender<any> = ({ children, render }, root) => { | ||
render(children, root); |
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.
this here is the actual fix right?
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.
Yes, the rest is just type updates.
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.
Thank you for the fix!
This PR fixes a bug when using the
renderer
option without therender
option.Since #920 you no longer need to pass the
render
option; instead you can pass a customrender
function via therenderer
option. This caused a bug with custom VDOM implementations becauserender
wasn't passed to the internaldefaultRender
function, and fell back on Preact'srender
.This PR also cleans up some tests that were never run. We can write them properly in a separate PR.