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

Strict null checks with arrays #13499

Closed
pgrm opened this issue Jan 15, 2017 · 6 comments
Closed

Strict null checks with arrays #13499

pgrm opened this issue Jan 15, 2017 · 6 comments
Labels
Working as Intended The behavior described is the intended behavior; this is not a bug

Comments

@pgrm
Copy link

pgrm commented Jan 15, 2017

TypeScript Version: 2.1.5 + http://www.typescriptlang.org/play/ both with strictNullChecks enabled

Code

// A *self-contained* demonstration of the problem follows...
interface ITest {
    key: string;
    values?: number[]
}

const t: ITest[] = [
    { key: 'key1', values: [] },
    { key: 'key2', values: [] }
];

for (const i of [1, 2, 3, 4, 5, 6, 7]) {
    t[0].values.push(i);
    t[1].values.push(i);
}

Expected behavior:

Since t[0] and t[1] have initialized values, I would expect that their type is number[]

Actual behavior:

I haven't found any workaround in which case t[0].values wouldn't be marked as possibly undefined anymore. Even if I set it right before the execution:

for (const i of [1, 2, 3, 4, 5, 6, 7]) {
    t[0].values = [];
    t[0].values.push(i);
}

I'm still getting the error that values might be undefined. My ugly workaround for now is to use 2 variables. Would it be possible to fix this in future versions of typescript?

@pgrm
Copy link
Author

pgrm commented Jan 15, 2017

Actually, using 2 variables I've found another (or similar?) issue:

Code

interface ITest {
    key: string;
    values?: number[]
}

const t1: ITest = { key: 'key1', values: [] };
const t2: ITest = { key: 'key2', values: [] };

for (const i of [1, 2, 3, 4, 5, 6, 7]) {
    t1.values.push(i);
    t2.values.push(i);
}

Expected behavior:

Since t1 and t2 have initialized values, I would expect that their type is number[]

Actual behavior:

values is still marked as potentially undefined, the following workaround fixes it:

const t1: ITest = { key: 'key1', values: [] };
const t2: ITest = { key: 'key2', values: [] };

t1.values = [];
t2.values = [];
for (const i of [1, 2, 3, 4, 5, 6, 7]) {
    t1.values.push(i);
    t2.values.push(i);
}

@mhegazy
Copy link
Contributor

mhegazy commented Jan 16, 2017

The type annotation makes the compiler not infer their value. so the type of t1 and t2 are both ITest and both have a values that is potentially undefined. removing the type annotation will allow the compiler to infer the type correctly.

@mhegazy mhegazy added the Working as Intended The behavior described is the intended behavior; this is not a bug label Jan 16, 2017
@pgrm
Copy link
Author

pgrm commented Jan 17, 2017

@mhegazy ok thank you, you're absolutely right and I should have known that. The reason I missed it is, if I remove the main type, I need to cast my 2 array and I end up with code like this:

// A *self-contained* demonstration of the problem follows...
interface ITest {
    key: string;
    values?: number[]
}

const t = [
    { key: 'key1', values: <number[]>[] },
    { key: 'key2', values: <number[]>[] }
];

for (const i of [1, 2, 3, 4, 5, 6, 7]) {
    t[0].values.push(i);
    t[1].values.push(i);
}

which I find quite ugly. Is there another, nicer, way to write this which I'm missing?

If not, even if this is not a bug in the current TypeScript version, wouldn't it be possible for a future version, to infer at least if optional types were set or not, and to which value union types were set. As an interface can be used to help define variables, figure out which properties I have to set, .... and the created object can be way more specific than the interface. This would of course just make sense for local variables.

@blakeembrey
Copy link
Contributor

blakeembrey commented Jan 17, 2017

I can't find the issue, but remember a proposal for something I believe would solve your pain. It would allow you to do:

interface ITest {
    key: string;
    values?: number[]
}

const t = [
    { key: 'key1', values: [] },
    { key: 'key2', values: [] }
] is ITest[];

for (const i of [1, 2, 3, 4, 5, 6, 7]) {
    t[0].values.push(i);
    t[1].values.push(i);
}

That above example just moves around ITest[] so that the compiler would continue inference, but the object could also be type checked without any coercion. If someone finds that issue, let me know 😄

@aluanhaddad
Copy link
Contributor

@blakeembrey I think this what you are looking for #7481 (comment)

@pgrm
Copy link
Author

pgrm commented Jan 18, 2017

thanks for the clarifications and solution 👍 I think everything has been said and referenced, for others to understand it later, so I'll close this issue again.

@pgrm pgrm closed this as completed Jan 18, 2017
@microsoft microsoft locked and limited conversation to collaborators Jun 19, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Working as Intended The behavior described is the intended behavior; this is not a bug
Projects
None yet
Development

No branches or pull requests

4 participants