Skip to content
This repository has been archived by the owner on Aug 31, 2021. It is now read-only.

Commit

Permalink
Generate (sound) TypeScript and Flow declaration files (#9)
Browse files Browse the repository at this point in the history
This change adds Flow declaration types so that we can check the types
of the generated code.

Notably:

- Adds a dependency to flowgen and calls it during compilation, which
  takes the `.d.ts` and creates a `.flow.js` based on it.
- Passes in the `--force-number` flag to pbjs, which causes it to not
  emit numbers as possibly being `Long`, since this confuses Flow. This
  only affects JSDoc declarations, not the generated code.
- Passes in the `--force-message` flag to pbjs. Otherwise, this emits
  invalid types: it tries to create code like this:

  ```TypeScript
  interface IMessage {
    field?: T|null;
  }

  class Message implements IMessage {
    field: T;  // notice the lack of optionality / nullness.
    ...
  }
  ```

  This violates the Liskov Substitution Principle, since `Message`
  cannot be passed to any place that accepts an `IMessage`, so
  TypeScript (correctly) rejects this as bogus. With this flag, the
  number of places that will accept an `IMessage` instead of a `Message`
  are limited to constructors, where we do want to have partial messages
  being passed in.

  This only affects JSDoc declarations, not the generated code.
- Removes all `@implements` annotations from the JSDoc, which takes care
  of the very last `IMessage`/`Message` confusion. This, together with
  the above element take care of
  protobufjs/protobuf.js#837
- Forwards calls from `.create()` to `.fromObject()` to avoid breaking existing
  code (w.r.t. types).
  • Loading branch information
lhchavez authored Dec 14, 2020
1 parent 4454663 commit 4e0ef13
Show file tree
Hide file tree
Showing 8 changed files with 15,169 additions and 1,584 deletions.
2 changes: 1 addition & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ build: build-go build-js
publish: publish-go publish-js

clean-js:
rm -f js/index.d.ts js/index.js
rm -f js/index.d.ts js/index.js js/export.flow.js
build-js: clean-js
cd js && npm install && npm run prepublishOnly
publish-js:
Expand Down
Loading

0 comments on commit 4e0ef13

Please sign in to comment.