Skip to content

non-strictNullChecks projects producing .d.ts files that cannot be used from a strictNullChecks projects #8995

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

Closed
rkirov opened this issue Jun 6, 2016 · 4 comments
Labels
Duplicate An existing issue was already created

Comments

@rkirov
Copy link
Contributor

rkirov commented Jun 6, 2016

TypeScript Version:

nightly (1.9.0-dev.20160217)

Code

// a.ts
export function f(foo: Foo): void {}

// b.ts
import {f} from './a';
f(null);

The produced 'a.d.ts' is:

export function f(foo: Foo): void;

Imagine a and b are different complication units and interface through a.d.ts. The code works as expected in TS 1.8.

Now imagine B decides to turn on strict null checks first.

Expected behavior:

The code should compile. B is stricter internally, but its compilation flags should not change the interpretations of existing .d.ts files.

Actual behavior:

b.ts is rejected because f in a.d.ts is interpreted as accepting only Foo, but really it is accepting Foo | null.

Vaguely remember an option for .d.ts containing a pragma was discussed, but I did not see it land, so I am not sure there are plans to work on that, or there are good reasons for not implementing it.

@mhegazy
Copy link
Contributor

mhegazy commented Jun 6, 2016

This has been discussed before in #8405.

I have a comment with reference to the paragma proposal in #8405 (comment)

@mhegazy mhegazy added the Duplicate An existing issue was already created label Jun 6, 2016
@rkirov
Copy link
Contributor Author

rkirov commented Jun 6, 2016

Thanks for the reference Mohammed. While issue is similar, there is a major difference with what I am describing here.

This is not "point-in-time migration request". The setup we have in google, works as follows, there are a number of projects - some are copies of external TS projects - angular2, RxJS, etc, and some are internal projects that depend on those. Each project (what I call a "compilation unit") above calls the TS compiler once. To speed up compilation and modularity, the project dependencies are not consumed through the original .ts files, but only through the resulting .d.ts files (produced by --declaration).

While we want all projects in our control to have strictNullChecks on, some external projects might never support that (or at least nothing we can do to make them switch). Then the situation above arises.

Say RxJs has to compile with strictNullChecks off, and produces rxjs.d.ts, saying:

function f(foo: Foo): void;

meaning my API accepts null and Foo.

Application A compiles against RxJs and wants to pass null to f.

f(null)

Because the rxjs.d.ts produced by --declaration is now used in a project that has 'strictNullChecks' on, the meaning of 'Foo' is changed to be 'non-null' only and f(null) does not compile.

The pragma indeed is a messy solution, maybe a better solution would be --strictNullDeclarations, which produces .d.ts files to be consumed by strictNullChecks enabled projects, while in the input is still considered non-strict.

@rkirov rkirov changed the title gradually enabling strictNullChecks between different compilation units non-strictNullChecks projects producing .d.ts files that cannot be used from a strictNullChecks projects Jun 6, 2016
@mhegazy
Copy link
Contributor

mhegazy commented Jun 6, 2016

the problem is function f(foo: Foo): void; in a non-strict-null project does not necessary mean it is Foo | null | undefined. it could mean that, or it could mean that it accepts only Foo, but since there was no strict null before, it was not checked strictly. so just saying that the declaration emit for foo: Foo here should be foo: Foo | undefined | null would be an overkill.

One thing to note, null and undefined are valid type names in non-strict-null mode. so function f(foo: Foo): void; could be written function f(foo: Foo | null): void; to support this scenario in both strict-null and loose-null modes.

I do still think this is a point-in-time still, an app can not really switch to strict-null until all its dependencies do, or at least be annotated in a strict-null-friendly fashion.

our analysis showed that in a typical .d.ts most of the declarations do hold in strict-null checks, i.e. function f(foo: Foo): void; really means Foo and not Foo | undefined | null. and that with some little work the .d.ts can be fully strict-null-compliant. this was the motivation to not add any additional layers of complexity by having a paragma. moreover, the paragma would have been only for error reporting, i.e. can not use a .d.ts unless it has the #ready for strict-null header, and that did not seem to add much value.

@aeschli
Copy link

aeschli commented Nov 9, 2017

+1 on this request. We need a way to mark which d.ts file are already strict-null check adopted, and which ones not.
We have so many dependencies, and it will take a long time for all of them to adopt their APIs to strict-nullness, if it happens at all. And it's not that we can easily modify a d.ts file coming from a dependency. Some are in node-module that we don't control.

@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
Duplicate An existing issue was already created
Projects
None yet
Development

No branches or pull requests

3 participants