-
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
🏷️ [#445] Add TS type definitions for Form/Submission #805
Conversation
Bundle ReportChanges will increase total bundle size by 464 bytes (0.0%) ⬆️. 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 ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #805 +/- ##
==========================================
- Coverage 83.55% 83.52% -0.03%
==========================================
Files 233 233
Lines 4633 4632 -1
Branches 1193 1193
==========================================
- Hits 3871 3869 -2
- Misses 732 733 +1
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:
|
This adds some of the type definitions for Form and Submission resources that we interact with from the v2 API endpoint(s), set up to strongly type the createSubmission function.
28e0581
to
bb36ea3
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.
Could we use something like https://www.npmjs.com/package/openapi-typescript for the API spec types? Otherwise i think updating the types will be a pain (and error-prone)..
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'm leaving out the bits that are not relevant for the SDK and are only used in the backend, otherwise it's confusing what the SDK needs to support and what is irrelevant. This is to declutter. So, assuming that openapi-typescript or similar supports this, it will still be maintenance heavy because you need to keep the allowlist in sync.
Honestly, if the API spec changes and you use something, it will break at runtime and should be caught by tests/storybook. If the API is expanded and you try to use it, the typescript compiler will complain about the type not being defined (correctly) and then you expand the type definitions.
I don't think there will much overhead from updating the types tbh.
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 prefer knowing (and having types for) entire objects. Just to have a clear overview of what i can expect. But I get wanting to keep it contained to just SDK specific stuff...
I've worked with one of such packages (not 100% sure if it was openapi-typescript, but it sounds very familiar), but there we could specify which object schemas to use when generating the types (which should help filtering to the information that is needed/interesting for the SDK).
And yeah, we will still have to update the SDK when the API spec changes, but this would cut back on the amount of double manual work..
But we can continue like this for now
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 prefer knowing (and having types for) entire objects.
Even that is tricky - the API spec also documents fields that are returned only to logged in administrators, but are not available to anonymous users getting the form details. We'd need a way to strip those out, because they are not present in the API responses made by the SDK :)
This adds some of the type definitions for Form and Submission resources that we interact with from the v2 API endpoint(s), set up to strongly type the createSubmission function.