Generate types#90
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR adds TypeScript type generation capabilities to the table-js library, following established patterns from diagram-js and bpmn-js. The changes include setting up tooling to generate types from source code and improving existing type definitions.
- Add type generation workflow using
bio-dtstool with testing capabilities - Update JSDoc type annotations to use proper TypeScript conventions
- Add generic type parameters to component classes for better type safety
Reviewed Changes
Copilot reviewed 10 out of 13 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| package.json | Adds type generation scripts, dependencies (bio-dts, cpx2, typescript), and updates CI workflow |
| .github/workflows/CI.yml | Updates diagram-js version dependency from 11 to 12 |
| src/features/editor-actions/EditorActions.js | Updates JSDoc type annotation from "Unknown" to "unknown" |
| src/features/context-menu/components/ContextMenuComponent.js | Restructures JSDoc parameter documentation for better type clarity |
| src/components/utils/mixin.js | Adds Component typedef import for better type definitions |
| src/components/BaseCell.js | Adds generic type parameters and constructor parameter types |
| src/components/HeaderCell.js | Adds generic type parameters and state type annotation |
| src/components/Cell.js | Adds generic type parameters and state type annotation |
| src/components/DiContainer.js | Adds generic type parameters with injector constraint |
| src/components/Cell.spec.tsx | Adds TypeScript test file demonstrating generic type usage |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
diagram-js<=11 does not offer type definitions, and hence the checks fail.
6d2d819 to
dd9234c
Compare
| "generate-types:generate": "del-cli \"{src,lib}/**/*.{d.ts,d.ts.map}\" && npx bio-dts --jsx -r src --outDir src && cpx \"src/**/*.{d.ts,d.ts.map}\" lib", | ||
| "generate-types:test": "tsc --noEmit --noImplicitAny", | ||
| "auto-build": "babel -s -d lib src --watch", | ||
| "prepublishOnly": "run-s build" |
There was a problem hiding this comment.
If we are to publish the types, they should be part of prepare or prepublish, e.g.
| "prepublishOnly": "run-s build" | |
| "prepare": "run-s build generate-types" |
Also, the types should be copied over to (or created directly in) the build directory (lib).
There was a problem hiding this comment.
Marked this as draft and will cut a pre-release next to aid testing against dmn-js.
Proposed Changes
This PR adds type definitions to this library, following the established "generation" pattern known from diagram-js and bpmn-js.
generate-typestasksrc, not fromliblibduring the process (libis what is being released to users)Related to bpmn-io/dmn-js#937
Checklist
To ensure you provided everything we need to look at your PR:
@bpmn-io/srtoolCloses {LINK_TO_ISSUE}orRelated to {LINK_TO_ISSUE}The question is if this is "ready to be released" by any means; there is many places where the types are incomplete / insufficient. To find that out I want to give the types a test run (release a pre-release, integrate with dmn-js).