-
-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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 ts types #1628
Fix ts types #1628
Conversation
@brentvatne this branch succeeds with |
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.
need to add back extract
for global properties
return ( | ||
<RNSVGCircle | ||
ref={this.refMethod} | ||
{...extract(this, props)} |
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 believe this would break existing functionality -- "global properties" would no longer apply. eg: you're meant to be able to use fill
with shapes/text. https://developer.mozilla.org/en-US/docs/Web/SVG/Attribute/fill
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.
Ok I'll fix that and add some tests as well
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.
@brentvatne I restored the extract(this, props)
in the affected elements. Testing with the SVG examples looks good. I'll try to do some snapshot tests to verify that all SVGs are rendered the same as before.
any idea why the package-lock.json diff is so huge, given that we're just updating two (not a big deal, just being more conservative here since i don't work on this repo) |
I had to delete it in order to run npm install. There were missing package files and conflicts if I didn't do that. |
Summary
Fix a large number of errors in TS type generation:
@types/react
and@types/react-native
src/elements
Test Plan
After the change,
npm install
succeeds in building the package.