-
Notifications
You must be signed in to change notification settings - Fork 637
fix: Builder Text node is preserved when converting to Mitosis JSON #1769
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
Merged
Merged
Changes from 27 commits
Commits
Show all changes
32 commits
Select commit
Hold shift + click to select a range
03b2780
add failing test
liamdebeasi 43fcf1e
remove Text component mapper
liamdebeasi 134be7f
include text in compile away buiulder components
liamdebeasi aa5c311
fix: remove Text transformations
liamdebeasi 8ee4e43
test: add another test
liamdebeasi d3de101
remove .only
liamdebeasi 6142822
sync
liamdebeasi cb75c4d
Merge branch 'main' into ENG-9431
liamdebeasi b1c9dd4
update spec
liamdebeasi f4f7a11
re-add single node merging
liamdebeasi 68e23b6
fix some bugs with merging nodes with single text nodes
liamdebeasi de4f9d2
fix: account for bindings too
liamdebeasi 1668ac8
fix tests
liamdebeasi 58d7fcd
fix a few tests
liamdebeasi df09a1d
fix a couple tests
liamdebeasi 8007538
tests
liamdebeasi ced2a98
update
liamdebeasi 001de8c
add todo
liamdebeasi 83cc9a4
remove old file
liamdebeasi 929f9c7
shiny seas do!
liamdebeasi cd699c8
Merge branch 'main' into ENG-9431
midhunadarvin 950e0fc
revert
liamdebeasi a38c9c8
remove log
liamdebeasi 69c5f1a
fix: styles are preserved when compiling away text node
liamdebeasi 36de61b
revert
liamdebeasi 425b75f
clean up
liamdebeasi b2a629b
fix test
liamdebeasi 1c7eb2e
bindings and props are rendered when compiling away Text node
liamdebeasi 9dafcd0
Merge branch 'main' into ENG-9431
liamdebeasi a3d4e05
types
liamdebeasi feb0a82
better sync
liamdebeasi 8e20873
more fixes
liamdebeasi File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,6 @@ | ||
| --- | ||
| '@builder.io/mitosis': minor | ||
| '@builder.io/mitosis-cli': minor | ||
| --- | ||
|
|
||
| feat: Builder Text node is no longer stripped away by default when converting to Mitosis |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
It looks like we lost bindings (class, style, "builder-id",
keyin another one below) in these tests. Do you know why? A bit concerningThere 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.
Good catch! This was a bug in my compile away implementation. Qwik does not render any bindings it sees on a node with
_text(since it's supposed to be plain text).I fixed the implementation by making sure bindings/props (except for _text) are rendered on the parent div: 1c7eb2e
However, this did uncover an inconsistency in the Qwik generator:
When a node has both an innerHTML binding and property, we prefer the binding. However, when the node has a
_textbinding and property we prefer the property.