feat: add type declarations for core components #732

philippfromme · 2023-01-17T09:10:16Z

Includes TypeScript declarations for

lib/command/*
lib/core/*
lib/model/*
lib/draw/BaseRenderer
lib/features/modeling/Modeling
lib/features/modeling/Overlays

Related to #78
Related to https://github.com/bpmn-io/internal-docs/issues/688

philippfromme · 2023-01-23T08:37:51Z

Next steps:

decide whether to add more type declarations (e.g. for context pad, palette, ...)
decide whether or not the strategy for testing the type declarations is sufficient
fix JSDoc comments
add type declarations to bpmn-js

nikku · 2023-01-24T18:47:34Z

lib/command/CommandHandler.d.ts

+  canExecute?(context: any): boolean;
+  preExecute?(context: any): void;
+  postExecute?(context: any): void;
+  $inject?: string[];


$inject is necessary? Why?

And if we require it it shall be static.

I was asking myself that question. I'd like to communicate somehow that a command handler is going to be instantiated by the command stack and, therefore, can use the $inject syntax. Unfortunately, interfaces do not support static members (cf. microsoft/TypeScript#13462).

Let's not transport that through types, but through proper documentation.

You could consider to expand the CommandHandler documentation or write new one (docs.bpmn.io).

nikku · 2023-01-24T21:51:03Z

Really nice stuff.

I dove into this and added #744 as a potential to improve this PR. Please have a 👀.

What I really like is how you created dedicated type specs (that also cover real world use-cases).

This way we can build out types on a real world use-case / demand basis.

nikku · 2023-01-24T21:51:28Z

Consider splitting up existing type tests so the file does not get HUGE at some point.

barmac · 2023-01-27T08:43:06Z

I'm looking into this today. Sorry for delay.

nikku · 2023-01-27T09:09:55Z

@barmac This is not urgent I believe. Please add your feedback, and also account for #744 in the process.

barmac · 2023-01-27T15:38:44Z

In the end, I deprioritized this for this week ^^

philippfromme · 2023-01-30T14:58:56Z

As discussed with @nikku we will:

merge Type declarations 2 #744 (review) into the feature branch of this pull request
not add more type declarations
add integration tests for each type declaration file (e.g. Canvas.spec.ts tests Canvas.d.ts)
fix and copy/add JSDoc comments to the type declarations

nikku · 2023-02-23T18:40:39Z

@philippfromme Looks like you're still making progress on this item.

Let's chat tomorrow what is left open / the status of this PR. Happy to review (if it makes sense) tomorrow.

philippfromme · 2023-02-23T18:51:33Z

@nikku I walked @marstamm through the pull request today and he'll review it, too. Yes, I'm making a couple of adjustments that are easy fixes. Let's chat tomorrow.

Fixes some type declarations.

@param

* fix @param types * fix formatting

philippfromme · 2023-02-28T08:30:07Z

@marstamm @nikku Let me know if there's anything keeping you from approving the pull request.

marstamm · 2023-02-28T08:45:50Z

I will look into it today

marstamm · 2023-02-28T13:49:32Z

lib/features/modeling/Modeling.spec.ts

+import CoreModule from '.';
+import ElementFactory from '../../core/ElementFactory';
+
+import ModelingModule from '.';
+import Modeling from './Modeling';


CoreModule and ModelingModule import the same thing here. Because the test suceeds, I assume we don't actually need the Core Module here?

I dropped it with 81d861e and it works fine

It is the same thing indeed 👍

Ooopsie. Good catch! The tests don't fail because errors due to missing modules can only happen during runtime.

marstamm

I simplified some import paths and fixed some typos. Can you confirm that #732 was just a copy/paste issue and not something fundamental?

Other than that, I am happy to merge it 🎉

nikku · 2023-03-01T07:34:00Z

Great stuff @philippfromme and great review @marstamm 🎉. Let's get this merged @philippfromme?

* options are optional * core module is added by default

philippfromme · 2023-03-01T08:31:53Z

I just added a last simplification. Let's merge!

philippfromme · 2023-03-01T08:36:00Z

🎉

bpmn-io-tasks bot added the in progress Currently worked on label Jan 17, 2023

bpmn-io-tasks bot assigned philippfromme Jan 17, 2023

philippfromme force-pushed the type-declarations branch 10 times, most recently from 899ed38 to 427577a Compare January 23, 2023 08:28

philippfromme requested review from nikku, barmac, marstamm and smbea January 23, 2023 08:35

philippfromme marked this pull request as ready for review January 23, 2023 08:46

bpmn-io-tasks bot added needs review Review pending and removed in progress Currently worked on labels Jan 23, 2023

philippfromme force-pushed the type-declarations branch from 427577a to 99e2517 Compare January 23, 2023 15:46

nikku reviewed Jan 24, 2023

View reviewed changes

nikku force-pushed the type-declarations branch from 99e2517 to b3f5935 Compare January 24, 2023 18:51

nikku added the in progress Currently worked on label Feb 1, 2023 — with bpmn-io-tasks

philippfromme force-pushed the type-declarations branch 4 times, most recently from a9ea7f6 to 6825273 Compare February 23, 2023 19:36

deps: update to [email protected]

6b86fd9

Fixes some type declarations.

philippfromme force-pushed the type-declarations branch 2 times, most recently from 897bc70 to e7d1a18 Compare February 24, 2023 07:59

philippfromme added 2 commits February 24, 2023 14:27

feat: add type declarations for core components

84d8f10

chore: fix JSDoc comments

9a9fc75

* fix @param types * fix formatting

philippfromme force-pushed the type-declarations branch from e7d1a18 to 9a9fc75 Compare February 24, 2023 13:28

philippfromme requested a review from nikku February 28, 2023 08:29

marstamm reviewed Feb 28, 2023

View reviewed changes

marstamm added 3 commits February 28, 2023 14:50

chore: simplify import paths

b225443

chore: rename CoreModule => OverlaysModule

04108e9

chore: drop unnecessary CoreModule

81d861e

marstamm reviewed Feb 28, 2023

View reviewed changes

chore: make diagram options optional

93e8aca

* options are optional * core module is added by default

marstamm approved these changes Mar 1, 2023

View reviewed changes

barmac approved these changes Mar 1, 2023

View reviewed changes

philippfromme merged commit 2ba1a7f into develop Mar 1, 2023

philippfromme deleted the type-declarations branch March 1, 2023 08:39

bpmn-io-tasks bot removed the in progress Currently worked on label Mar 1, 2023

philippfromme mentioned this pull request Mar 2, 2023

chore(CHANGELOG): update to v11.10.0 #754

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: add type declarations for core components #732

feat: add type declarations for core components #732

philippfromme commented Jan 17, 2023 •

edited

Loading

philippfromme commented Jan 23, 2023 •

edited

Loading

nikku Jan 24, 2023

nikku Jan 24, 2023

philippfromme Jan 30, 2023

nikku Jan 30, 2023

nikku commented Jan 24, 2023

nikku commented Jan 24, 2023

barmac commented Jan 27, 2023

nikku commented Jan 27, 2023

barmac commented Jan 27, 2023

philippfromme commented Jan 30, 2023 •

edited

Loading

nikku commented Feb 23, 2023 •

edited

Loading

philippfromme commented Feb 23, 2023

philippfromme commented Feb 28, 2023

marstamm commented Feb 28, 2023

marstamm Feb 28, 2023

marstamm Feb 28, 2023

nikku Mar 1, 2023

philippfromme Mar 1, 2023 •

edited

Loading

marstamm left a comment

nikku commented Mar 1, 2023

philippfromme commented Mar 1, 2023

philippfromme commented Mar 1, 2023

feat: add type declarations for core components #732

feat: add type declarations for core components #732

Conversation

philippfromme commented Jan 17, 2023 • edited Loading

philippfromme commented Jan 23, 2023 • edited Loading

nikku Jan 24, 2023

Choose a reason for hiding this comment

nikku Jan 24, 2023

Choose a reason for hiding this comment

philippfromme Jan 30, 2023

Choose a reason for hiding this comment

nikku Jan 30, 2023

Choose a reason for hiding this comment

nikku commented Jan 24, 2023

nikku commented Jan 24, 2023

barmac commented Jan 27, 2023

nikku commented Jan 27, 2023

barmac commented Jan 27, 2023

philippfromme commented Jan 30, 2023 • edited Loading

nikku commented Feb 23, 2023 • edited Loading

philippfromme commented Feb 23, 2023

philippfromme commented Feb 28, 2023

marstamm commented Feb 28, 2023

marstamm Feb 28, 2023

Choose a reason for hiding this comment

marstamm Feb 28, 2023

Choose a reason for hiding this comment

nikku Mar 1, 2023

Choose a reason for hiding this comment

philippfromme Mar 1, 2023 • edited Loading

Choose a reason for hiding this comment

marstamm left a comment

Choose a reason for hiding this comment

nikku commented Mar 1, 2023

philippfromme commented Mar 1, 2023

philippfromme commented Mar 1, 2023

philippfromme commented Jan 17, 2023 •

edited

Loading

philippfromme commented Jan 23, 2023 •

edited

Loading

philippfromme commented Jan 30, 2023 •

edited

Loading

nikku commented Feb 23, 2023 •

edited

Loading

philippfromme Mar 1, 2023 •

edited

Loading