WIP use TypeScript unknown in isolate
#873
Conversation
staltz
force-pushed
the
isolate-unknown
branch
from
1e990a6
to
860a91d
Compare
| @@ -440,7 +440,7 @@ describe('isolate', function() { | |||
| } | |||
|
|
|||
| function MyDataflowComponent( | |||
| sources: {other: any}, | |||
| sources: {other: unknown}, | |||
There was a problem hiding this comment.
Even before this PR, this line should have been unknown.
But now, with this PR, this test does not pass (type check) with any (because scopedSinks.other would have been inferred as unknown, which would be a compile error a few lines below), so I had to change it to unknown.
| @@ -492,11 +492,11 @@ describe('isolate', function() { | |||
| {other: new MyTestSource()}, | |||
| `foo`, | |||
| `bar` | |||
| ); | |||
| ) as {other: Observable<Array<string>>}; | |||
There was a problem hiding this comment.
If Isolate were aware of Most.js and RxJS types of streams, we could support this test to type check without any changes. So we have to manually cast to RxJS's stream type here, otherwise it would be unknown.
| @@ -531,11 +531,11 @@ describe('isolate', function() { | |||
| {other: new MyTestSource()}, | |||
| `foo`, | |||
| `bar` | |||
| ); | |||
| ) as {other: Observable<Array<number>>}; | |||
| @@ -624,15 +624,15 @@ describe('isolate', function() { | |||
| }; | |||
| } | |||
|
|
|||
| function MyDataflowComponent(sources: {other: any}) { | |||
| function MyDataflowComponent(sources: {other: unknown}) { | |||
There was a problem hiding this comment.
Same case as this comment
| @@ -142,7 +142,7 @@ describe('withState', function() { | |||
| function Parent(sources: ParentSources): ParentSinks { | |||
| const childSinks = isolate(Child, {state: 'child'})(sources); | |||
| return { | |||
| state: childSinks.state, | |||
| state: childSinks.state as Stream<Reducer<Parent>>, | |||
There was a problem hiding this comment.
This case is a true example of an inference that we don't yet know how to support. Previously Isolate was inferring these cases as either Stream<Reducer<any>> or Stream<Reducer<{}>>, with this PR, the type is Stream<unknown> so it breaks on purpose unless the developer does a type cast, like we're doing here.
BREAKING CHANGE: If you use JavaScript, there are no breaking changes. If you use TypeScript, then from now onwards isolate() returns sink types that are never type "any", but instead returns "unknown". Often it may infer the correct type, but when it cannot infer the type, it will return "unknown" which means you are forced to type cast it to the correct type. This is better than "any", because "any" is like JavaScript and gives little type safety.
staltz
force-pushed
the
isolate-unknown
branch
from
860a91d
to
ff698ce
Compare
Do not merge yet
pnpm run commitinstead ofgit commitThis PR just shows what kind of breaking changes would happen if we would introduce TypeScript's
unknowninstead ofany. The purpose is to avoid sneakinganytypes that tend to virally propagate, and instead to useunknownwhich forces the developer to cast the type of some sinks returned from an isolated component. In other words,isolatewould never giveanytypes, it would force the developer to think about what type they expect.Open for discussion before I do the following: