-
Notifications
You must be signed in to change notification settings - Fork 2
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 hierarchy request error with empty yield #31
base: main
Are you sure you want to change the base?
Fix hierarchy request error with empty yield #31
Conversation
@jembezmamy thanks for adding this! To get this merged, we'll need to make sure your commits are verified: let me know if the instructions of github are enough to solve it, otherwise we can do this together. |
{{yield}} | ||
{{/if}} | ||
</Tag> | ||
{{#let (this.or @hasBlock (has-block)) as |normalizedHasBlock|}} |
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.
Looks like a good patch to solve the problem we're facing with wrapping the bridge and yielding content. The original fix (using has-block
) was already a patch. Long term, I think we'll need to fix the root cause in the modifier.
Could you add it as an issue in github so we'll keep track of it?
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 log it in the existing issue #30
I fixed commit signing so now it's ready for merge |
Hey @SkoebaSteve, can we merge this? |
This PR adds a
@hasBlock
argument toReactBridge
allowing to bypass the empty block error described in issue #30. When the argument is empty,(has-block)
helper will be used - same as before. So the change is backward-compatible.I added a failing test demonstrating the issue. It happens only when an empty blocked is passed and an argument is updated, causing an re-render.