-
Notifications
You must be signed in to change notification settings - Fork 33
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
feat: add type declarations to distro #65
base: main
Are you sure you want to change the base?
Conversation
I think this improvement will make it easier to work with moddle when developing extensions for bpmn-js. |
6646aae
to
df78b1c
Compare
@MartyDashP Thanks for your contribution 🚀 This PR fits well into our ongoing initiatives to offer types for our libraries. As a matter of fact, exported types are a contract, an we need to ensure that we have tests for them; understand when they break accidentally, but also to show proposed use (example). Could you investigate a way how to add such tests for the generated types? Some learnings from typing some of our core libraries (diagram-js, bpmn-js):
|
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.
We need test coverage for any types that we generate. Added some thoughts.
@nikku Thank for the feedback. I'll try to cover all exported types with tests. I can't promise that it will be done quickly, but I will do my best 😀 |
9720389
to
acfd390
Compare
@MartyDashP I had a look at the PR, good bunch of TS magic! Find some adjustments pushed onto the branch, including proper integration into the overall library build + publishing. I see that you heavily use A few follow-ups we'd need if we wanted to incorporate it into the library:
Probably next step is to get these real world examples incorporated (tests against |
@nikku I tried to cover all the public APIs with tests and hopefully took into account all the mentioned comments 🙂 |
Thanks for your updates. Hope to be able to look into this PR again, this week. |
Co-authored-by: Nico Rehwaldt <git_nikku@nixis.de>
@nikku 👋 Were you able to review my pull request? Do you have any comments or suggestions? 😀 |
Proposed Changes
Add type declarations to distribution package
Checklist
To ensure you provided everything we need to look at your PR:
@bpmn-io/sr
toolCloses {LINK_TO_ISSUE}
orRelated to {LINK_TO_ISSUE}