MithrilJS / mithril.d.ts

Types for mithril.js
MIT License
80 stars 11 forks source link

Breaking `Stream` constructor when using optional value #38

Closed ketan closed 5 years ago

ketan commented 5 years ago

The following typescript that used to work earlier, stopped working after upgrade:

import Stream from "mithril/stream";

class Foo {
  id: Stream<string>;
  constructor(id?: string) {
    this.id = Stream(id);
  }
}

I understand v2 is a breaking change, I'm just trying to understand if it's an intentional breaking change. The change was introduced in commit 65ec92942792018534c341a985ce159fc6f9c2a0, where this line was moved here with a change in signature.

The fix is fairly simple, before I submit a PR, I thought I'd check if this was intentional, and if this fix is acceptable?

diff --git a/stream/index.d.ts b/stream/index.d.ts
index 1702f10..c47e250 100644
--- a/stream/index.d.ts
+++ b/stream/index.d.ts
@@ -1,7 +1,7 @@
 /** Creates an empty stream. */
 declare function Stream<T = any>(): Stream<T>;
 /** Creates a stream with an initial value. */
-declare function Stream<T>(value: T): Stream<T>;
+declare function Stream<T>(value?: T): Stream<T>;

 declare interface Stream<T> {
    /** Returns the value of the stream. */
spacejack commented 5 years ago

Hi @ketan. This change was intentional and it's actually pointing out a type error. You've declared id as Stream<string> however you're trying to set its value to string | undefined. So what you'd want to do is declare id as:

id: Stream<string | undefined>;
ketan commented 5 years ago

Thank you for confirming. Closing.