Skip to content
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

Merged
merged 8 commits into from
Mar 11, 2025

Conversation

sergei-maertens
Copy link
Member

Partly closes #445

The latter is typescript compatible.
Copy link

codecov bot commented Mar 6, 2025

Bundle Report

Changes will decrease total bundle size by 1.97kB (-0.02%) ⬇️. This is within the configured threshold ✅

Detailed changes
Bundle name Size Change
@open-formulieren/sdk-OpenForms-umd 4.9MB -984 bytes (-0.02%) ⬇️
@open-formulieren/sdk-esm 4.87MB -988 bytes (-0.02%) ⬇️

Affected Assets, Files, and Routes:

view changes for bundle: @open-formulieren/sdk-esm

Assets Changed:

Asset Name Size Change Total Size Change (%)
assets/sdk-*.js -982 bytes 1.02MB -0.1%
assets/index-*.js -6 bytes 45.46kB -0.01%

Files in assets/sdk-*.js:

  • ./src/components/AppDebug.tsx → Total Size: 1.52kB

  • ./src/components/Loader.tsx → Total Size: 672 bytes

  • ./src/components/Price.tsx → Total Size: 849 bytes

  • ./src/env.ts → Total Size: 536 bytes

  • ./src/components/FAIcon.jsx → Total Size: 734 bytes

  • ./src/api.ts → Total Size: 4.55kB

  • ./src/components/AppDisplay.tsx → Total Size: 928 bytes

  • ./src/components/PreviousLink.jsx → Total Size: 682 bytes

  • ./src/utils.ts → Total Size: 696 bytes

  • ./src/components/Anchor/Anchor.jsx → Total Size: 898 bytes

  • ./src/components/Caption.tsx → Total Size: 245 bytes

  • ./src/components/forms/SelectField/SelectField.jsx → Total Size: 4.96kB

  • ./src/components/Errors/ErrorBoundary.jsx → Total Size: 4.89kB

view changes for bundle: @open-formulieren/sdk-OpenForms-umd

Assets Changed:

Asset Name Size Change Total Size Change (%)
open-*.js -984 bytes 3.53MB -0.03%

Files in open-*.js:

  • ./src/components/AppDebug.tsx → Total Size: 1.52kB

  • ./src/formio/components/Component.js → Total Size: 2.17kB

  • ./src/components/PreviousLink.jsx → Total Size: 682 bytes

  • ./src/components/Anchor/Anchor.jsx → Total Size: 898 bytes

  • ./src/components/Errors/ErrorBoundary.jsx → Total Size: 4.89kB

  • ./src/components/AppDisplay.tsx → Total Size: 928 bytes

  • ./src/components/FAIcon.jsx → Total Size: 734 bytes

  • ./src/components/Price.tsx → Total Size: 849 bytes

  • ./src/components/forms/SelectField/SelectField.jsx → Total Size: 4.97kB

  • ./src/env.ts → Total Size: 536 bytes

  • ./src/api.ts → Total Size: 4.55kB

  • ./src/components/Loader.tsx → Total Size: 672 bytes

  • ./src/utils.ts → Total Size: 696 bytes

  • ./src/components/Caption.tsx → Total Size: 245 bytes

Copy link

codecov bot commented Mar 6, 2025

Codecov Report

Attention: Patch coverage is 91.52542% with 5 lines in your changes missing coverage. Please review.

Project coverage is 83.53%. Comparing base (19fc530) to head (7dc4a84).
Report is 9 commits behind head on main.

Files with missing lines Patch % Lines
src/api.ts 88.88% 3 Missing ⚠️
src/utils.ts 87.50% 2 Missing ⚠️
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              
Flag Coverage Δ
storybook 76.86% <88.13%> (+0.04%) ⬆️
vitest 62.27% <80.39%> (+0.09%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@sergei-maertens sergei-maertens marked this pull request as ready for review March 7, 2025 08:17
Copy link
Contributor

@robinmolen robinmolen left a 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
@sergei-maertens sergei-maertens force-pushed the chore/445-convert-to-typescript branch from 1131b47 to 5c22554 Compare March 10, 2025 13:40
Copy link
Contributor

@robinmolen robinmolen left a 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.
@sergei-maertens sergei-maertens force-pushed the chore/445-convert-to-typescript branch from 5c22554 to 7dc4a84 Compare March 11, 2025 07:18
@sergei-maertens sergei-maertens merged commit 575138f into main Mar 11, 2025
17 checks passed
@sergei-maertens sergei-maertens deleted the chore/445-convert-to-typescript branch March 11, 2025 07:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Adopt typescript
2 participants