Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Non-nullable types support for TypeScript 2.0 #914

Closed
haizz opened this issue Jul 11, 2016 · 9 comments
Closed

Non-nullable types support for TypeScript 2.0 #914

haizz opened this issue Jul 11, 2016 · 9 comments

Comments

@haizz
Copy link

haizz commented Jul 11, 2016

Since a new version of TypeScript with --strictNullCheck will be released soon it would be cool to have support for this in Immutable.js typings.

microsoft/TypeScript#7140

@pavadeli
Copy link
Contributor

pavadeli commented Jul 13, 2016

Something like this? #919 is a start, but full non-nullable types support needs more work I'm afraid.

@OliverJAsh
Copy link

#919 was merged, but this only fixes the closure parameters. The remaining issue is with the return types:

listOfNumbers.get(5) // returns 5, should return 5 | undefined

What work is involved to get this done? Is it simply a case of updating the type definitions?

@haizz
Copy link
Author

haizz commented Jan 30, 2017

This might get tricky because arrayOfNumbers[index] has type number even if arrayOfNumbers is number[] (!), and this is intentional (microsoft/TypeScript#9235) although it's at least quite controversial if you ask me.

const array: number[] = [];

array[5000] = 5;

const a = array[0]; // a is a `number`, not `number | undefined` !!!

const b = List().toArray();

// **EDIT**: this is not actually accurate for JavaScript:
// b.forEach(v => { /* v has `number` type but can have `undefined` value! */ } );
// b.map(v => { /* v has `number` type but can have `undefined` value! */ } );
// ...

So one has to decide should Immutable.js mimic this behavior or should it have more type safety?
If we choose type safety then which way? Make all methods like forEach, find, map, sortBy, ... to go with T | undefined for values because Immutable.js iterates over undefined after List resize? mapper: (v: number | undefined, k: number | undefined) => M for Set<number>? Does it make any sense? What about toArray() and toObject()?

Or should we instead change all mutation methods which may potentially introduce undefined values to return List<T | undefined>? Like set, setSize, or List constructor:

function of<T>(...values: T[]): List<T>;
function List<T>(): List<T>;
function List<T>(array: Array<T>): List<T | undefined>;

interface List<T> extends Collection.Indexed<T> {
    // ...
    push(...values: T[]): List<T>; // push can't add undefined values, so we keep List<T> type
    set(index: number, value: T): List<T | undefined>; // can't guarantee that new list won't have undefined values
    setSize(size: number): List<T | undefined>; // can't guarantee that new list won't have undefined values
}

I don't know...

EDIT: I wasn't quite accurate: JavaScript actually does not iterate over undefined values if one does a[50000] = 1, and Immutable.js behaves in a different way. Still there's a question about type of value parameters in iteration methods.

@OliverJAsh
Copy link

So one has to decide should Immutable.js mimic this behavior or should it have more type safety?

Interestingly, TypeScript's built-in signature for ES6 Map and Set does include undefined in the get signature: https://github.com/Microsoft/TypeScript/blob/445421b68b23a3fcb1a64d9541f31b5e0131ed34/src/lib/es2015.collection.d.ts#L5

@haizz
Copy link
Author

haizz commented Feb 4, 2017

All of this looks quite messy to me. For example, forEach of Javascript array and forEach of Immutable.js List have different semantics:

const array: number[] = [];
array[3] = 5;
array.forEach(v => console.log(v));
// Output:
// 5

vs

const list = List<number>().set(3, 5);
list.forEach(v => console.log(v));

// Output:
// undefined
// undefined
// undefined
// 5

I don't see a particularly good way to make it both null-safe and not annoying to use in real life. Any ideas?

@haizz
Copy link
Author

haizz commented Feb 4, 2017

Another issue: Immutable.js is using .d.ts file to build documentation by using custom TypeScript syntax processor which is based on TypeScript version 1.3 embedded into the source tree (https://github.com/facebook/immutable-js/blob/master/pages/third_party/typescript/package.json). Obviously it breaks down when I add | undefined types because TS 1.3 doesn't even support union types yet! Just plugging in modern TS 2.1 won't simply work because doc generator is based on TypeScript.SyntaxWalker which apparently has been removed from TS (microsoft/TypeScript#1728).

TL;DR: you can't just upgrade typings with union types, non-nulls, tuple types, this types, etc. without breaking documentation generation.

@jbrownson
Copy link
Contributor

Looks like a lot of this is already in git and not released, noticed thanks to #1034

@leebyron
Copy link
Collaborator

leebyron commented Mar 3, 2017

New type definitions are almost done, and will be released soon!

@leebyron leebyron closed this as completed Mar 3, 2017
@MicahZoltu
Copy link

@leebyron What does "soon" mean in this context? I'm getting TS compiler errors because the definition for predicates have nullable parameters, when in reality they don't. If a release is coming in a day or two, I'll wait. If not, I'll have to figure out a way to manually bring the updated definitions into my project.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants