-
Notifications
You must be signed in to change notification settings - Fork 4
Issues fix - Chore/migrate render html #12
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
base: chore/migrate-render-html
Are you sure you want to change the base?
Issues fix - Chore/migrate render html #12
Conversation
translateX: 'block' | ||
} as never; |
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.
I don't think as never
is an acceptable tradeoff; I would rather not lose type safety here.
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.
I adjusted the type settings - and I'm already implementing the support for missing style attributes, I thought that it would be benefitial for me to learn how it is implemented
packages/render-html/package.json
Outdated
"@native-html/transient-render-engine": "workspace:*", | ||
"@native-html/transient-render-engine": "^11.2.3", |
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 monorepo is meant to use Yarn workspaces; we cannot really avoid that. Having to manually handle versions would be cumbersome!
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.
Okay, I brought back previous implementation
94165d2
to
b5405f2
Compare
@jsamr I implemented it so that it supports props that are working (and should be supported I suppose), and ignores those that are either not working or should not be supported ( |
b5405f2
to
512c1b0
Compare
@5ZYSZ3K Thanks! Will review this weekend. Please press "re-request review" so that I get notified when you think it's ready. |
Fixed all of issues mentioned under the original PR
Also tested the migrated library in one of my private projects - was working fine