-
Notifications
You must be signed in to change notification settings - Fork 6
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
Convert some low-hanging fruit to TypeScript #804
Conversation
The latter is typescript compatible.
Bundle ReportChanges will decrease total bundle size by 1.97kB (-0.02%) ⬇️. This is within the configured threshold ✅ Detailed changes
Affected Assets, Files, and Routes:view changes for bundle: @open-formulieren/sdk-esmAssets Changed:
Files in
view changes for bundle: @open-formulieren/sdk-OpenForms-umdAssets Changed:
Files in
|
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #804 +/- ##
==========================================
+ Coverage 83.42% 83.53% +0.10%
==========================================
Files 233 233
Lines 4633 4633
Branches 1180 1193 +13
==========================================
+ Hits 3865 3870 +5
+ Misses 738 733 -5
Partials 30 30
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
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.
Some comments are questions if we want to be strict/specific with the typing. Just take a look and let me know you're thoughts
And immediately there is a type issue where strings are passed down to FormattedNumber, which is invalid.
Explicit @/ imports are preferred as they deliberately signal that it's code from our own root/src dir. This will eventually allow us to clean up the rest of the import sorting config.
Type checker reported bugs: * subtracting dates from each other (sort-of false positive...) * the way we handle params/headers in API calls didn't account for null * some parseInt call had the radix _outside_ of the function call
1131b47
to
5c22554
Compare
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've added one final comment, but it's not very important. It can be discussed/addressed or you can merge it, both are fine 😄
Dropped the value prop in favour of always passing children.
5c22554
to
7dc4a84
Compare
Partly closes #445