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 optional properties #43947

Merged
merged 18 commits into from
Jun 1, 2021
Merged

Strict optional properties #43947

merged 18 commits into from
Jun 1, 2021

Conversation

ahejlsberg
Copy link
Member

@ahejlsberg ahejlsberg commented May 4, 2021

With this PR we introduce a new strict mode, --strictOptionalProperties, in which optional properties declared without an explicit undefined in their type do not permit assigning a possibly undefined value. This makes it possible to ensure that optional properties are never undefined when they're present on an object, and that undefined values read from optional properties unequivocally indicate that the properties are not present. The new compiler switch is in the --strict family of switches and this PR is therefore a breaking change for code compiled in --strict mode (unless, of course, the switch is explicitly set to false).

// Compile in --strictOptionalProperties mode
function f1(obj: { a?: string, b?: string | undefined }) {
    let a = obj.a;  // string | undefined
    let b = obj.b;  // string | undefined
    obj.a = 'hello';
    obj.b = 'hello';
    obj.a = undefined;  // Error, 'undefined' not assignable to 'string'
    obj.b = undefined;
}

In --strictOptionalProperties mode, assigning undefined to obj.a is considered an error because the declared type of a doesn't explicitly include undefined. However, when reading obj.a the type is still string | undefined to account for the possibility that the object doesn't have an a property.

The b property above explicitly includes undefined in its type and is thus unaffected by --strictOptionalProperties.

In --strictOptionalProperties mode, for an optional property declared without undefined in its type, control flow analysis accepts the in operator and the hasOwnProperties method as proof that the property is present, and consequently excludes undefined from the property value.

// Compile in --strictOptionalProperties mode
function f2(obj: { a?: string }) {
    obj = obj;
    obj.a = obj.a;  // Error,  'string | undefined' not assignable to 'string'
    if ('a' in obj) {
        obj.a = obj.a;  // Ok
    }
    if (obj.hasOwnProperty('a')) {
        obj.a = obj.a;  // Ok
    }
}

Note above that obj.a is not assignable to itself unless it is guarded by a check that the property is present. While initially surprising, this makes sense because, if the property is not present, it would become present with the value undefined following the assignment.

In --strictOptionalProperties mode, the Partial<T> utility type produces strictly checked optional properties as expected. For example:

// Compile in --strictOptionalProperties mode
function f3(obj: Partial<{ a: string, b: string | undefined }>) {
    let a = obj.a;  // string | undefined
    let b = obj.b;  // string | undefined
    obj.a = 'hello';
    obj.b = 'hello';
    obj.a = undefined;  // Error, 'undefined' not assignable to 'string'
    obj.b = undefined;
}

In --strictOptionalProperties mode, optional elements in tuple types are strictly checked when they don't explicitly include undefined in their type. For example:

// Compile in --strictOptionalProperties mode
function f4(t: [string?]) {
    let x = t[0];  // string | undefined
    t[0] = 'hello';
    t[0] = undefined;  // Error, 'undefined' not assignable to 'string'
}

Fixes #13195.

@ahejlsberg
Copy link
Member Author

@typescript-bot pack this

@typescript-bot
Copy link
Collaborator

typescript-bot commented May 4, 2021

Heya @ahejlsberg, I've started to run the tarball bundle task on this PR at a01fd4f. You can monitor the build here.

@ahejlsberg ahejlsberg added this to the TypeScript 4.4.0 milestone May 4, 2021
@ahejlsberg
Copy link
Member Author

@typescript-bot test this
@typescript-bot user test this
@typescript-bot run dt
@typescript-bot perf test this

@typescript-bot
Copy link
Collaborator

typescript-bot commented May 4, 2021

Heya @ahejlsberg, I've started to run the parallelized Definitely Typed test suite on this PR at a01fd4f. You can monitor the build here.

@typescript-bot
Copy link
Collaborator

typescript-bot commented May 4, 2021

Heya @ahejlsberg, I've started to run the extended test suite on this PR at a01fd4f. You can monitor the build here.

@typescript-bot
Copy link
Collaborator

typescript-bot commented May 4, 2021

Heya @ahejlsberg, I've started to run the perf test suite on this PR at a01fd4f. You can monitor the build here.

Update: The results are in!

@typescript-bot
Copy link
Collaborator

typescript-bot commented May 4, 2021

Heya @ahejlsberg, I've started to run the parallelized community code test suite on this PR at a01fd4f. You can monitor the build here.

@typescript-bot
Copy link
Collaborator

Hey @ahejlsberg, I've packed this into an installable tgz. You can install it for testing by referencing it in your package.json like so:

{
    "devDependencies": {
        "typescript": "https://typescript.visualstudio.com/cf7ac146-d525-443c-b23c-0d58337efebc/_apis/build/builds/102249/artifacts?artifactName=tgz&fileId=CFB344EF3C397BA1D95AD763E0788A023222649B2EDD04CB666F2541D8CA138B02&fileName=/typescript-4.3.0-insiders.20210504.tgz"
    }
}

and then running npm install.

@typescript-bot
Copy link
Collaborator

@ahejlsberg
The results of the perf run you requested are in!

Here they are:

Comparison Report - master..43947

Metric master 43947 Delta Best Worst
Angular - node (v10.16.3, x64)
Memory used 345,029k (± 0.03%) 345,014k (± 0.02%) -15k (- 0.00%) 344,859k 345,119k
Parse Time 1.93s (± 0.50%) 1.93s (± 0.49%) +0.00s (+ 0.21%) 1.91s 1.95s
Bind Time 0.84s (± 0.41%) 0.84s (± 0.79%) +0.01s (+ 0.72%) 0.83s 0.86s
Check Time 5.28s (± 0.41%) 5.27s (± 0.58%) -0.01s (- 0.17%) 5.22s 5.34s
Emit Time 5.95s (± 0.68%) 5.91s (± 0.74%) -0.04s (- 0.64%) 5.86s 6.07s
Total Time 13.99s (± 0.43%) 13.96s (± 0.48%) -0.03s (- 0.24%) 13.86s 14.19s
Compiler-Unions - node (v10.16.3, x64)
Memory used 200,829k (± 0.03%) 200,787k (± 0.03%) -41k (- 0.02%) 200,658k 200,957k
Parse Time 0.78s (± 0.87%) 0.79s (± 0.92%) +0.00s (+ 0.51%) 0.77s 0.81s
Bind Time 0.53s (± 0.90%) 0.53s (± 1.64%) +0.00s (+ 0.19%) 0.51s 0.55s
Check Time 7.59s (± 0.23%) 7.61s (± 0.74%) +0.02s (+ 0.21%) 7.46s 7.71s
Emit Time 2.50s (± 1.02%) 2.50s (± 1.20%) -0.01s (- 0.24%) 2.42s 2.56s
Total Time 11.41s (± 0.27%) 11.42s (± 0.62%) +0.02s (+ 0.14%) 11.25s 11.53s
Monaco - node (v10.16.3, x64)
Memory used 341,707k (± 0.02%) 341,683k (± 0.02%) -24k (- 0.01%) 341,508k 341,828k
Parse Time 1.57s (± 0.45%) 1.57s (± 0.60%) +0.00s (+ 0.06%) 1.54s 1.59s
Bind Time 0.75s (± 0.94%) 0.75s (± 0.89%) -0.00s (- 0.27%) 0.73s 0.76s
Check Time 5.38s (± 0.43%) 5.40s (± 0.54%) +0.01s (+ 0.26%) 5.34s 5.46s
Emit Time 3.04s (± 0.86%) 3.01s (± 0.58%) -0.03s (- 1.05%) 2.97s 3.05s
Total Time 10.74s (± 0.48%) 10.72s (± 0.43%) -0.02s (- 0.20%) 10.61s 10.81s
TFS - node (v10.16.3, x64)
Memory used 304,254k (± 0.03%) 304,289k (± 0.02%) +35k (+ 0.01%) 304,108k 304,391k
Parse Time 1.22s (± 0.67%) 1.22s (± 0.51%) +0.00s (+ 0.16%) 1.20s 1.23s
Bind Time 0.71s (± 0.78%) 0.71s (± 0.78%) +0.00s (+ 0.56%) 0.70s 0.72s
Check Time 4.82s (± 0.40%) 4.82s (± 0.42%) -0.01s (- 0.15%) 4.78s 4.87s
Emit Time 3.21s (± 1.46%) 3.15s (± 1.50%) -0.05s (- 1.65%) 3.04s 3.28s
Total Time 9.95s (± 0.52%) 9.90s (± 0.60%) -0.06s (- 0.56%) 9.75s 9.99s
material-ui - node (v10.16.3, x64)
Memory used 474,654k (± 0.02%) 477,528k (± 0.02%) +2,874k (+ 0.61%) 477,355k 477,652k
Parse Time 1.94s (± 0.75%) 1.95s (± 0.51%) +0.00s (+ 0.15%) 1.92s 1.97s
Bind Time 0.65s (± 0.95%) 0.66s (± 1.14%) +0.01s (+ 0.92%) 0.64s 0.67s
Check Time 14.86s (± 0.64%) 15.59s (± 0.76%) +0.73s (+ 4.90%) 15.42s 15.83s
Emit Time 0.00s (± 0.00%) 0.00s (± 0.00%) 0.00s ( NaN%) 0.00s 0.00s
Total Time 17.45s (± 0.58%) 18.19s (± 0.68%) +0.74s (+ 4.22%) 18.02s 18.45s
Angular - node (v12.1.0, x64)
Memory used 322,550k (± 0.04%) 322,575k (± 0.04%) +24k (+ 0.01%) 322,337k 322,863k
Parse Time 1.92s (± 0.49%) 1.92s (± 0.62%) +0.01s (+ 0.26%) 1.89s 1.95s
Bind Time 0.83s (± 1.16%) 0.82s (± 0.83%) -0.00s (- 0.48%) 0.81s 0.84s
Check Time 5.17s (± 0.35%) 5.18s (± 0.46%) +0.01s (+ 0.14%) 5.11s 5.21s
Emit Time 6.16s (± 0.67%) 6.16s (± 0.80%) +0.00s (+ 0.03%) 6.07s 6.30s
Total Time 14.07s (± 0.36%) 14.09s (± 0.47%) +0.01s (+ 0.10%) 13.92s 14.25s
Compiler-Unions - node (v12.1.0, x64)
Memory used 187,800k (± 0.21%) 187,704k (± 0.19%) -96k (- 0.05%) 186,719k 188,517k
Parse Time 0.77s (± 1.01%) 0.77s (± 0.67%) +0.00s (+ 0.26%) 0.76s 0.78s
Bind Time 0.53s (± 0.89%) 0.53s (± 0.89%) +0.00s (+ 0.57%) 0.52s 0.54s
Check Time 7.05s (± 0.79%) 7.07s (± 0.59%) +0.02s (+ 0.26%) 6.97s 7.15s
Emit Time 2.50s (± 1.27%) 2.50s (± 1.26%) -0.00s (- 0.12%) 2.42s 2.57s
Total Time 10.85s (± 0.71%) 10.87s (± 0.57%) +0.02s (+ 0.20%) 10.76s 11.00s
Monaco - node (v12.1.0, x64)
Memory used 324,060k (± 0.03%) 324,109k (± 0.03%) +49k (+ 0.01%) 323,950k 324,356k
Parse Time 1.54s (± 0.66%) 1.54s (± 0.88%) +0.00s (+ 0.00%) 1.51s 1.57s
Bind Time 0.72s (± 0.80%) 0.72s (± 0.41%) -0.00s (- 0.28%) 0.71s 0.72s
Check Time 5.21s (± 0.44%) 5.20s (± 0.62%) -0.00s (- 0.04%) 5.14s 5.27s
Emit Time 3.08s (± 0.80%) 3.08s (± 0.55%) +0.00s (+ 0.00%) 3.02s 3.10s
Total Time 10.54s (± 0.41%) 10.54s (± 0.38%) +0.00s (+ 0.01%) 10.42s 10.63s
TFS - node (v12.1.0, x64)
Memory used 288,738k (± 0.03%) 288,752k (± 0.02%) +14k (+ 0.00%) 288,598k 288,948k
Parse Time 1.22s (± 0.71%) 1.23s (± 0.86%) +0.00s (+ 0.41%) 1.20s 1.24s
Bind Time 0.70s (± 0.72%) 0.69s (± 0.86%) -0.00s (- 0.14%) 0.68s 0.71s
Check Time 4.71s (± 0.37%) 4.73s (± 0.31%) +0.01s (+ 0.28%) 4.69s 4.76s
Emit Time 3.19s (± 0.63%) 3.17s (± 1.00%) -0.02s (- 0.63%) 3.09s 3.26s
Total Time 9.82s (± 0.34%) 9.82s (± 0.51%) -0.00s (- 0.05%) 9.69s 9.92s
material-ui - node (v12.1.0, x64)
Memory used 452,295k (± 0.07%) 455,344k (± 0.05%) +3,049k (+ 0.67%) 454,422k 455,513k
Parse Time 1.95s (± 0.59%) 1.95s (± 0.65%) -0.00s (- 0.20%) 1.93s 1.98s
Bind Time 0.64s (± 0.62%) 0.64s (± 0.81%) 0.00s ( 0.00%) 0.63s 0.65s
Check Time 13.37s (± 0.95%) 14.11s (± 0.72%) +0.74s (+ 5.55%) 13.93s 14.39s
Emit Time 0.00s (± 0.00%) 0.00s (± 0.00%) 0.00s ( NaN%) 0.00s 0.00s
Total Time 15.96s (± 0.78%) 16.70s (± 0.67%) +0.74s (+ 4.62%) 16.50s 17.01s
Angular - node (v14.15.1, x64)
Memory used 321,273k (± 0.01%) 321,233k (± 0.01%) -40k (- 0.01%) 321,123k 321,286k
Parse Time 1.90s (± 0.61%) 1.92s (± 0.79%) +0.02s (+ 1.10%) 1.88s 1.95s
Bind Time 0.87s (± 0.66%) 0.87s (± 0.94%) +0.00s (+ 0.34%) 0.86s 0.90s
Check Time 5.18s (± 0.42%) 5.19s (± 0.54%) +0.01s (+ 0.27%) 5.12s 5.25s
Emit Time 6.25s (± 0.65%) 6.30s (± 0.49%) +0.05s (+ 0.85%) 6.23s 6.37s
Total Time 14.20s (± 0.40%) 14.29s (± 0.30%) +0.10s (+ 0.68%) 14.21s 14.38s
Compiler-Unions - node (v14.15.1, x64)
Memory used 188,819k (± 0.61%) 189,496k (± 0.50%) +677k (+ 0.36%) 186,895k 190,200k
Parse Time 0.81s (± 0.94%) 0.81s (± 0.59%) +0.00s (+ 0.12%) 0.80s 0.82s
Bind Time 0.56s (± 0.80%) 0.56s (± 0.93%) +0.00s (+ 0.54%) 0.55s 0.57s
Check Time 7.17s (± 0.69%) 7.21s (± 0.68%) +0.04s (+ 0.63%) 7.09s 7.30s
Emit Time 2.48s (± 0.59%) 2.50s (± 0.74%) +0.02s (+ 0.69%) 2.45s 2.54s
Total Time 11.01s (± 0.54%) 11.07s (± 0.60%) +0.06s (+ 0.59%) 10.91s 11.17s
Monaco - node (v14.15.1, x64)
Memory used 323,155k (± 0.01%) 323,177k (± 0.00%) +22k (+ 0.01%) 323,146k 323,198k
Parse Time 1.57s (± 0.55%) 1.58s (± 0.55%) +0.01s (+ 0.64%) 1.56s 1.60s
Bind Time 0.75s (± 0.66%) 0.75s (± 0.94%) +0.00s (+ 0.40%) 0.73s 0.76s
Check Time 5.20s (± 0.38%) 5.22s (± 0.41%) +0.02s (+ 0.40%) 5.16s 5.25s
Emit Time 3.13s (± 0.90%) 3.12s (± 0.56%) -0.01s (- 0.32%) 3.08s 3.16s
Total Time 10.65s (± 0.41%) 10.67s (± 0.32%) +0.02s (+ 0.23%) 10.59s 10.75s
TFS - node (v14.15.1, x64)
Memory used 287,649k (± 0.01%) 287,682k (± 0.01%) +33k (+ 0.01%) 287,646k 287,717k
Parse Time 1.26s (± 1.15%) 1.27s (± 0.68%) +0.01s (+ 0.88%) 1.25s 1.29s
Bind Time 0.72s (± 0.81%) 0.72s (± 1.08%) +0.00s (+ 0.28%) 0.71s 0.74s
Check Time 4.72s (± 0.45%) 4.74s (± 0.64%) +0.02s (+ 0.40%) 4.70s 4.81s
Emit Time 3.29s (± 0.52%) 3.28s (± 0.50%) -0.00s (- 0.09%) 3.25s 3.31s
Total Time 9.99s (± 0.30%) 10.01s (± 0.50%) +0.03s (+ 0.26%) 9.91s 10.12s
material-ui - node (v14.15.1, x64)
Memory used 450,714k (± 0.01%) 453,632k (± 0.06%) +2,919k (+ 0.65%) 452,501k 453,838k
Parse Time 2.01s (± 0.63%) 2.00s (± 0.87%) -0.01s (- 0.45%) 1.94s 2.03s
Bind Time 0.71s (± 1.65%) 0.70s (± 0.95%) -0.00s (- 0.28%) 0.68s 0.71s
Check Time 13.52s (± 0.63%) 14.33s (± 0.88%) +0.80s (+ 5.94%) 14.14s 14.77s
Emit Time 0.00s (± 0.00%) 0.00s (± 0.00%) 0.00s ( NaN%) 0.00s 0.00s
Total Time 16.23s (± 0.62%) 17.03s (± 0.77%) +0.80s (+ 4.90%) 16.85s 17.47s
System
Machine Namets-ci-ubuntu
Platformlinux 4.4.0-206-generic
Architecturex64
Available Memory16 GB
Available Memory8 GB
CPUs4 × Intel(R) Core(TM) i7-4770 CPU @ 3.40GHz
Hosts
  • node (v10.16.3, x64)
  • node (v12.1.0, x64)
  • node (v14.15.1, x64)
Scenarios
  • Angular - node (v10.16.3, x64)
  • Angular - node (v12.1.0, x64)
  • Angular - node (v14.15.1, x64)
  • Compiler-Unions - node (v10.16.3, x64)
  • Compiler-Unions - node (v12.1.0, x64)
  • Compiler-Unions - node (v14.15.1, x64)
  • Monaco - node (v10.16.3, x64)
  • Monaco - node (v12.1.0, x64)
  • Monaco - node (v14.15.1, x64)
  • TFS - node (v10.16.3, x64)
  • TFS - node (v12.1.0, x64)
  • TFS - node (v14.15.1, x64)
  • material-ui - node (v10.16.3, x64)
  • material-ui - node (v12.1.0, x64)
  • material-ui - node (v14.15.1, x64)
Benchmark Name Iterations
Current 43947 10
Baseline master 10

Developer Information:

Download Benchmark

@tom-sherman
Copy link

tom-sherman commented May 4, 2021

I'm getting unexpected type information when playing around with this example from the linked issue. Does this require further work in the language server to rectify?

image

I'd expect for the type of foo to be foo?: string on hover.

type InputProps = {
  foo?: string;
  bar: string;
}

// Hover over foo here
const inputProps: InputProps = { foo: undefined, bar: 'bar' };

@typescript-bot
Copy link
Collaborator

The user suite test run you requested has finished and failed. I've opened a PR with the baseline diff from master.

@Andarist
Copy link
Contributor

Andarist commented May 5, 2021

I love the change and couldn't be happier about it ❤️

I have a minor question/concern though: what about types coming from external dependencies? narrowing down based on the in operator or hasOwnProperty might not be sound in that case, right? Since the external dependency could be compiled without this strict flag.

@ahejlsberg
Copy link
Member Author

@tom-sherman In the new --strictOptionalProperties mode, for optional properties we have two possible types we could display: The type you get when reading the property, or the type you're permitted to write to the property. You can observe the difference by hovering over each foo in the the statement inputProps.foo = inputProps.foo ?? ''. In your particular example I agree it makes more sense to display the type you're permitted to write.

@m-rutter
Copy link

m-rutter commented May 5, 2021

I love the change and couldn't be happier about it heart

I have a minor question/concern though: what about types coming from external dependencies? narrowing down based on the in operator or hasOwnProperty might not be sound in that case, right? Since the external dependency could be compiled without this strict flag.

Also think about definitely typed. I don't think anyone is going to individually verify that every js implementation will obey the new strict version of ?:? Maybe it might make sense to do a blanket | undefined codemod to definitely typed as a one off migration?

Edit: maybe that is a bit over dramatic

@OliverJAsh
Copy link
Contributor

@ahejlsberg I expected this example would now compile but it doesn't seem to unfortunately. Do you know why that is?

type Dict<T> = { [key: string]: T };
interface F extends Dict<string> {
    // Property 'foo' of type 'string | undefined' is not assignable to string index type 'string'.
    foo?: string;
}

@simeyla
Copy link

simeyla commented Jul 14, 2021

Excited about this option and glad to see it officially in TS 4.4 :-)

Just a suggestion for docs, that you may want to remind people they can use the delete operator to remove a property (since exactOptionalPropertyTypes of course means you can't set it undefined). I feel like a lot of people probably don't know about this operator - or need reminding about it if they're accustomed to setting things undefined. I think it would be important to mention in any documentation on this new flag.

interface Person {
    name: string,
    age?: number;
}

const person: Person = {
    name: "Daniel",
    age: 27
};

p.age = undefined;    // Error! undefined isn't a number

Instead, to clear the property with the exactOptionalPropertyTypes flag enabled use:

delete person.age;

Caveat: At this point in time this gives an error (playground link) which I'm assuming is a temporary issue?

Also: Despite the URL parameter in this playground link, you must manually enable exactOptionalPropertyTypes under tsoptions to turn iton (does that mean I found two bugs)

@DanielRosenwasser
Copy link
Member

DanielRosenwasser commented Jul 14, 2021

@simeyla maybe that's a suggestion worth bringing into @sandersn's change at #45032. When directly assigning undefined to an existing property, we can give some diagnostic that suggests using delete.

On the other hand, I don't know how much we recommend that pattern in the first place, and we might be more inclined to tell users to add undefined to the property type.

Edit: regarding your caveat, just try it out in a nightly version. There is not a second bug, exactOptionalPropertyTypes is off by default, even under --strict.

@sandersn
Copy link
Member

@simeyla Actually, can you open a second bug? #45032 covers a slightly different error (values with properties that are undefined) and is pretty big already. Just copy your example into a new bug -- it captures the request precisely. At that point we can decide whether to handle it in the same codefix or not.

@simeyla
Copy link

simeyla commented Jul 15, 2021

@DanielRosenwasser @sanders

  1. Looks like the issue is already taken care of in the nightly build. After I posted this I actually found a test case for exactly this that presumably must have been failing and was since fixed.

  2. I've created an issue Guidance / doc improvements for exactOptionalPropertyTypes for deleting an optional property that cannot be set undefined. #45058

  3. I realize that this isn't under --strict but it's literally in the URL parameters for the above playground link - and isn't being respected when I reload the page.

    eg. https://www.typescriptlang.org/play?ts=4.4.0-dev.20210715& exactOptionalPropertyTypes=true #code

@sandersn
Copy link
Member

@simeyla can you file another bug about the playground bug and assign it to @orta?

@craigphicks
Copy link

craigphicks commented Sep 8, 2021

I see in the source code the following related intrinsic types being used:

        const undefinedType = createIntrinsicType(TypeFlags.Undefined, "undefined");
        const optionalType = createIntrinsicType(TypeFlags.Undefined, "undefined");
        const missingType = exactOptionalPropertyTypes ? createIntrinsicType(TypeFlags.Undefined, "undefined") : undefinedType;

They are not exported. -> missingType is not exported. Looking at types through the compiler API lens of getTypeOfSymbolAtLocation() and getDeclaredTypeAtLocation(), and using (ts.Type.flags) to distinguish between types, all those types appear to be the same. That leads to loss of functionality.

Ah! The optionalType is already exported from TypeChecker (if ts-expose-interals is used).

checker.getOptionalType()

missingType is not exported presumably because it intended never to persist outside of checker internal computation. However there is one place in code where is it set. Under checkArrayLiteral:

                else if (exactOptionalPropertyTypes && e.kind === SyntaxKind.OmittedExpression) {
                    hasOmittedExpression = true;
                    elementTypes.push(missingType);   //// <--------------- can this missingType leave the nest?  
                    elementFlags.push(ElementFlags.Optional);
                }
                else {
                    const elementContextualType = getContextualTypeForElementExpression(contextualType, elementTypes.length);
                    const type = checkExpressionForMutableLocation(e, checkMode, elementContextualType, forceTuple);
                    elementTypes.push(addOptionality(type, /*isProperty*/ true, hasOmittedExpression));
                    elementFlags.push(hasOmittedExpression ? ElementFlags.Optional : ElementFlags.Required);
                }               

Is it possible that missingType might persist? Is it intended never to persist? If so, it should probably be exported with a checker.getMissingType(), shouldn't it?

Actually missingType does persist - an example:

type T1 = [x?:boolean]

produces, through the API checker.getDeclaredTypeOfSymbol a tuple with a single member which is a union of a true type, a false type, and missingType. From the API pov, a checker function getMissingType() is needed to correctly identity that.

@kokushkin
Copy link

Thanks for the strictOptionalProperties, but it seems to me now that it leads to a lot of redundancy that looks like fieldName?: TypeName | undefined in all over the codebases. For ex. https://github.com/DefinitelyTyped/DefinitelyTyped/pull/54352/files and so forth and so on. It makes code more complicated to read.

This happens because the authors of the libraries have no clue about the state of the strictOptionalProperties in the project that uses it, therefore they assume "the worst" (which is strictOptionalProperties: true) and have to write fieldName?: TypeName | undefined.

The problem is that in the most popular scenario, which is when people don't care about whether fieldName exists or undefined, it leads to the most wordy expression fieldName?: TypeName | undefined, should be quite opposite, isn't?

I don't know what were the alternatives. If the only problem was an impossibility to say "the field may not exist, but must not be undefined" (is there any "real" situation when people actually needed it, by the way? not Foo?), then fieldName?: TypeName && not undefined might be a better option? (I know it might be also complicated) . I wonder if there were other options that were considered and refused?

@MartinJohns
Copy link
Contributor

then fieldName?: TypeName && not undefined might be a better option?

TypeScript has no way to represent a "not"-type. This would require #29317.

@ivan-kleshnin
Copy link

ivan-kleshnin commented Oct 22, 2021

@kokushkin that's the consequence of adding TS features that were never intented to be present in the language.

People pushed for this one and noUncheckedIndexedAccess, they were added... and how they are easier to avoid than to use. TS dev. team warned about this right at the start but we didn't listen. 🙉

@craigphicks
Copy link

craigphicks commented Oct 22, 2021

@kokushkin - It's worth pointing out that the change (1) didn't break existing (without the "strictOptionalProperties") setups, (2) the .d.ts files can, and probably were, changed progmaticaly with a script, and (3) without the change some native javascript scenarios simply could not be represented in Typescript, fundamentally hobbling Typescript.

Apart from the readability issue (which IMO is not a problem), the only problem is old apps without "strictOptionalProperties" being extended to use new libs with "strictOptionalProperties", because the app could try to pass a disallowed "undefined" to a lib that doesn't accept it, and there would be no UI warning. (I am guessing there is no warning).

If so, then maybe information could be placed in the .d.ts file to inform that it is assuming "strictOptionalProperties", and typescript could generate a warning for case of app with "strictOptionalProperties:false" and libs with "strictOptionalProperties:true".

@Offirmo
Copy link

Offirmo commented Mar 21, 2022

Hi, I'm coming here from the docs and I'm not sure about the value of this property. It seems to be "true | false | undefined" but it's not stated clearly in the doc: https://www.typescriptlang.org/tsconfig#exactOptionalPropertyTypes

Also which is the strictest option? Is it "true" ?

@Andarist
Copy link
Contributor

It defaults to false and if u want this strict behavior then u can opt into it using true

@Kay-Yuan
Copy link

Kay-Yuan commented Oct 7, 2022

Hi, I tried exactOptionalPropertyTypes flag on my Angular project. It works fine in my .ts file. The optional property type is type-example only without undefined. But the .html file still showed the optional property type as type-example | undefined. Do you guys have any idea about this? Any feedback would be appreciated.

@MartinJohns
Copy link
Contributor

@Kay-Yuan That's an issue with Angular and should be addressed there. TypeScript has nothing to do with .html files.

@Kay-Yuan
Copy link

Kay-Yuan commented Oct 7, 2022

@Kay-Yuan That's an issue with Angular and should be addressed there. TypeScript has nothing to do with .html files.

Thanks a lot! Just not sure which part of the error belong. I will comment this to Angular. Cheers.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Author: Team Breaking Change Would introduce errors in existing code For Backlog Bug PRs that fix a backlog bug
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

Distinguish missing and undefined