Improve type hints to support advanced code navigation and assistance in IDEs (Intellij, VSCode) and LSPs

[philippfromme]

What should we do

We should fix the JSDoc type definitions across our projects and leverage TypeScript's support for JavaScript projects.

• Fix type definitions on services and service APIs (Fix JSDoc types for services diagram-js#501, Fix JSDoc type definitions on service APIs diagram-js#502, Fix JSDoc types for services #1378, Fix JSDoc type definitions on service APIs #1379)
• Verify correct usage of types via type linting tsc --noEmit --pretty
• Introduce actual type definitions for core entities (i.e. the model), unless they can be safely consumed from JS
• Consider publishing types generated from JSDoc (Provide Documentation #332 (comment))
• Consider strategies to type our DI container bpmnJS.get
• Consider generating type definitions from our source files to aid type support in bundled setups

What we should not do

We should not, as of today, migrate our projects to TypeScript:

• Type checking JavaScript projects and correct JSDoc on JavaScript projects provides equal capabilities and DX on the consumer side.
• Not transpiling our code base during development (and run) is a major benefit.

[nikku]

@philippfromme I've commented with a few more findings.

@barmac Would be amazing if you could fill in the TypeScript blanks 😉.

[lppedd]

@nikku I'll work with this project using Typescript, thus I will most likely produce type definitions for it.
There is currently a request for it here.

My opinion is that you should move everything to Typescript incrementally. I've noticed it really gives a boost to code quality and organization.

Also, as a user of IDEA/WebStorm, I've yet to find a better tooling, it's amazing, no joke.

[nikku]

@lppedd Thanks for sharing your feedback.

Just to manage expectations: We cannot promise any move to TypeScript any time soon 😉 .

[nikku]

As mentioned in the issue text, there exists other solution approaches that will likely give you the same amount of type hints without the need for us to move fully to TypeScript. We'd appreciate your help improving the typed support.

[lppedd]

@nikku I almost completed typings for BpmnJS and EventBus. I would just love to have more documentation on the objects returned to the callback functions, which seems to change between event types.

[lppedd]

@nikku @philippfromme here you can find an initial attempt at typings. I've done a bit of everything, but still I'm losing a lot of time debugging trying to understand objects' structures. If you could give a look and comment with some suggestion, it would be awesome!

The nice thing about the declarations is that didi instances are typed, e.g.

const elementRegistry = this.bpmn.get('elementRegistry');
      ^ type is ElementRegistry

ElementRegistry is then "fully" typed

export default class ElementRegistry {
  constructor(eventBus: EventBus);
  add(element: Base, gfx: SVGElement, secondaryGfx?: SVGElement): void;
  remove(element: string | Base): void;
  updateId(element: string | Base, newId: string): void;
  get(filter: string | SVGElement): Base;
  filter(fn: (element: Base, gfx: SVGElement) => boolean): Base[];
  getAll(): Base[];
  forEach(fn: (element: Base, gfx: SVGElement) => any): void;
  getGraphics(filter: string | Base, secondary?: boolean): SVGElement;
}

[nikku]

I like this one in particular that implements your example.

[lppedd]

@nikku yeah, that was a good idea! The same concept is applied also to Moddle#create here.
Example:

const startEvent = moddle.create('bpmn:StartEvent');
      ^ type is BpmnStartEvent

const extensionElements= moddle.create('bpmn:ExtensionElements');
      ^ type is BpmnExtensionElements

I've provided "defaults" for bpmn-js, but a user can extend those typings by simply redefining BpmnElementMap (here).
These means I can do, in my module

declare module 'moddle/lib/moddle' {
   interface BpmnElementMap {
      'myns:CustomElement': MyCustomElement;
   }
}

and then, in implementation code

const myElement = moddle.create('myns:CustomElement', { ... });
      ^ type is MyCustomElement

const definitions = moddle.create('bpmn:Definitions', { ... });
      ^ type is BpmnDefinitions

And if no match

const whatever = moddle.create('whatever');
      ^ type is simply ModdleElement

What I'm not sure about is the ModdleElement hierarchy here.
I think I'm doing something wrong.
What I'm trying to do is not use the any type as much as possible, because that's just like plain JS.

[lppedd]

That's how I implemented didi injection. I throw this in here because it might be useful for future reference.

@Inject(
  'eventBus',
  'styles',
  'pathMap',
  'canvas',
  'textRenderer'
)
class CustomRenderer extends BpmnRenderer {
  constructor(
    eventBus: EventBus,
    ...

The @Inject decorator is pretty simple

export function Inject(injection: string, ...others: string[]) {
  return <T extends Ctor>(ctor: T): void => {
    ctor.$inject = [injection, ...others];
  };
}

Which is basically what you'd do with

CustomRenderer.$inject = [
  'eventBus',
  'styles',
  'pathMap',
  'canvas',
  'textRenderer'
];

Just a lot cleaner for TS.

[yfwz100]

This is awesome! Is it planned to release in npm?

[nikku]

Nothing planned here.

[patoncrispy]

It would be great to see detailed API documentation and/or TypeScript added to this project. It is quite a lot of effort to find out what properties certain objects have (contexts, elements, events, etc.) what events are available, what parts of the system respond to which events, what the 'best practices' are for using/overriding/creating modules, what modules do, among other things.

This would be particularly useful when breaking changes are introduced, as while there is a changelog, type safety really does help the developer experience.

I would be happy to help out with at least the TypeScript effort if there is to be one.

[lppedd]

@nikku Hi! I've made some progress with typings. I'd like some help with this declaration, the context parameter.
https://github.com/lppedd/bpmnio-typings/blob/cc78174f2ff610b9d410928d1f2be779ec431f8b/src/%40types/diagram-js/index.d.ts#L1226

[lppedd]

@patoncrispy If you want you can PR here https://github.com/lppedd/bpmnio-typings for now.
I'd like to move it to DefinitelyTyped but it's still early imho.

[patoncrispy]

Cool thanks @lppedd I'll take a look soon.

[patoncrispy]

Hey @lppedd what more work do you think would need to be done to get your typings work into DefinitelyTyped?

[lppedd]

@patoncrispy it has been months I haven't worked on TS/JS projects, but I can assure you what I have done is a small portion of the framework. There is much much more

[nikku]

For historical purposes, this is what we've found in the past:

---

Option 1: Leverage TypeScript foundations

...

Option 2: Leverage existing JSDoc annotations

Right now we have a quite comprehensive set of existing type definitions, baked into our JSDoc statements. One example from our code base (Viewer#importXML):

/**
 * Parse and render a BPMN 2.0 diagram.
 *
 * Once finished the viewer reports back the result to the
 * provided callback function with (err, warnings).
 *
 * ## Life-Cycle Events
 *
 * During import the viewer will fire life-cycle events:
 *
 *   * import.parse.start (about to read model from xml)
 *   * import.parse.complete (model read; may have worked or not)
 *   * import.render.start (graphical import start)
 *   * import.render.complete (graphical import finished)
 *   * import.done (everything done)
 *
 * You can use these events to hook into the life-cycle.
 *
 * @param {String} xml the BPMN 2.0 xml
 * @param {ModdleElement<BPMNDiagram>|String} [bpmnDiagram] BPMN diagram or id of diagram to render (if not provided, the first one will be rendered)
 * @param {Function} [done] invoked with (err, warnings=[])
 */
Viewer.prototype.importXML = function(xml, bpmnDiagram, done) {
...

This allows VSCode to provide decent hints already. It does not know about ModdleElement though, as we do not declare it.

Idea: Create type definitions for all types we refer to

By providing the corresponding JSDoc type definitions we can improve the existing developer experience. Given the following typings:

typeDefs.js

/**
 * @typedef Bounds
 * @type {object}
 * @property {number} x
 * @property {number} y
 * @property {number} width
 * @property {number} height
 */

/**
 * @typedef TRBL
 * @type {object}
 * @property {number} top
 * @property {number} right
 * @property {number} bottom
 * @property {number} left
 */

... and the following feature:

MyFeature.js

import 'typeDefs';

class Foo {

  /**
   * @param {Bounds}
   */
  foo(bounds) {
    // ...
  }
}

... the assistance in VSCode (IntelliSense) can properly provide full information about fooFeature#foo:

import FooFeature from 'FooFeature';

class BarFeature {
  
  /**
   * @param {FooFeature} fooFeature
   */
  constructor(fooFeature) {
    fooFeature.foo(); // IntelliSense to the rescue!
  }
}

A better way of importing things (seen in TypeScript, too) is by actually importing the used types from somewhere and use them in the documentation tags, only:

import { Bounds } from 'diagram-js-types';

class Foo {

  /**
   * @param {Bounds}
   */
  foo(bounds) {
    // ...
  }
}

Areas of Research

• How to properly declare inherits relationships
• How to properly declare types for utilities, i.e. Bounds
• Does importing types for typing support only impact bundle sizes (initial research shows no, as these import declarations are being tree-shaken)

[nikku]

As indicated via #332 (comment) you can now generate TypeScript definitions from JSDoc types.

[philippfromme]

Closed by https://github.com/bpmn-io/internal-docs/issues/688. bpmn-js@13 includes type declarations.