GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — add compiler flag to enable granular inference outside var/let by KiaraGrouwstra · Pull Request #17785 · microsoft/TypeScript

Conversation

@KiaraGrouwstra

@KiaraGrouwstra kiara (KiaraGrouwstra) commented Aug 14, 2017 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor

This is a new compiler flag (idea: #16389 (comment)) to cancel type widening for objects / arrays (and even infer those as tuples) in const declarations as well as parameters:

// @widenTypes: false 
let a = [1, 2, 3];
// as before: number[]
a.push(4);
// ok
const b = [1, 2, 3];
// now [1, 2, 3]
b.push(4);
// boom
let c = { a: true };
// as before: { a: boolean }
const d = { a: true };
// now { a: true }

This would fix inference issues that spawned #3369, #6574, #7799, #8276, #9216, #9217, #10195, #12308, #12955, #15656, #16276, #16389, #16523, #16656, #17181. (Probably not exhaustive, just followed some links.)
Some of those propose alternate solutions like new keywords, though I presume this would address their issues as well.

Sometimes one may want granular types, sometimes one may want widened types. And in parameter positions, unfortunately only one of the two can be default, so either will end up wrong in some cases.
For declarations this holds as well, but the appeal of this flag there is that it yields users more control, based on whether they use the let or const keyword.

Notes:

  • Yes, it overloads the semantics of these JS keywords, if in a way that makes a surprising amount of sense.
  • it feels a bit unfortunate I had to propagate this boolean across 20 methods, should their params (new one and checkMode, or even the nodes too) be standardized into some options objects?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

piotrwitek, pelotom, bykbtzr, gcnew, ssonne, andnp, kujon, haltcase, bcherny, frenic, and 4 more reacted with thumbs up emoji anurbol reacted with thumbs down emoji arturkulig, ssonne, alvis, and WhiteAbeLincoln reacted with heart emoji
All reactions
  • 👍 14 reactions
  • 👎 1 reaction
  • ❤️ 4 reactions
@msftclas Microsoft Contribution License Agreements (msftclas) added the cla-already-signed label Aug 14, 2017
@KiaraGrouwstra kiara (KiaraGrouwstra) mentioned this pull request Aug 14, 2017
suggestion: explicit "tuple" syntax #16656
Open
@ajafff Klaus Meinhardt (ajafff) mentioned this pull request Aug 14, 2017
Refine no-unnecessary-type-assertion rule to not flag necessary assertions palantir/tslint#3120
Merged
This was referenced Aug 15, 2017
loosen number index check, fixes #15768 #17767
Merged
Make tuples have known length #17765
Merged
@dead-claudia

Claudia Meadows (dead-claudia) commented Aug 15, 2017

Copy link
Copy Markdown

Question: does this affect either of theses? If so, you'll need to add a special case to allow them, or you'll need an ungodly number of casts for things like options objects, etc.

interface SomeInterface { a: boolean }
declare function foo(arg: number[]): void
declare function bar(arg: SomeInterface): void

// As variable assignees
const a: number[] = [1, 2, 3];
const b: SomeInterface = {a: true};

// Same, but as arguments
foo([1, 2, 3]);
bar({a: true});
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@KiaraGrouwstra

kiara (KiaraGrouwstra) commented Aug 15, 2017 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor Author

@isiahmeadows:

// @widenTypes: false 

interface SomeInterface { a: boolean }
declare function foo(arg: number[]): void
declare function bar(arg: SomeInterface): void

// As variable assignees
const a: number[] = [1, 2, 3]; // number[]
const b: SomeInterface = {a: true}; // SomeInterface

// Same, but as arguments
foo([1, 2, 3]); // ok, tuple assignable to array
bar({a: true}); // ok, assignable

const a: number[] = [1, 2, 3];

I thought of still typing a as the more granular inferred type as well, as for people it's convenient to annotate with a short-hand, while the compiler is better at inferring the real value. And while the granular version is at least as powerful, that might work. Tuples would lack the ability to meaningfully use mutating methods though, so it's kinda up in the air:

  • have the user-annotated number[] override trump, allowing control even within const.
  • figure out a way to ensure tuple types will be full-fledged array types (assignable / featuring their mutating methods).

I'm kinda neutral there, I presume things will gravitate toward option 1.

you'll need to add a special case to allow them, or you'll need an ungodly number of casts for things like options objects, etc.

Well, { a: true } is assignable to { a: boolean }.
Otherwise this idea would be nothing but terrible, yeah.

Edit: updated.

aluanhaddad, dead-claudia, and piotrwitek reacted with thumbs up emoji
All reactions
  • 👍 3 reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@KiaraGrouwstra
kiara (KiaraGrouwstra) force-pushed the 16656-granularConst branch from 4bea1a9 to 601f5f2 Compare August 17, 2017 05:35
@KiaraGrouwstra

kiara (KiaraGrouwstra) commented Aug 17, 2017 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor Author

Okay, got parameters down too.

Now, this was dubbed a huge / massive breaking change, so... what's the damage?
I decided to try this on TS and all of its tests to find out:

  • obviously, since granular types become default for const and parameter positions, use let or explicit annotations/casts if you want mutability.
  • similarly, for functions like <T>(a: T, b: T) it becomes the users responsibility to ensure a widened type is passed as the first param or generic. If T becomes an array, then tuples should be widened in either position, though if Make tuples have known length #17765 brings array-tuple assignability that would be resolved.

That's it; couple dozen casts on a humongous code-base.

So the good news is, this doesn't even depend on #17765; it seems good now. I'm ready for feedback.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@KiaraGrouwstra
add widenTypes flag
c85ba7b
@KiaraGrouwstra
kiara (KiaraGrouwstra) force-pushed the 16656-granularConst branch from 7b9bf2c to c85ba7b Compare August 17, 2017 06:26
@KiaraGrouwstra
update spec
a6dc21d
This was referenced Aug 24, 2017
fleshing out type operators (discussion) #16392
Closed
Typing for object deep paths #12290
Closed
Syntax for hinting literal type inference #10195
Closed
Empty tuple type now meaningful (at least, in unions) #13126
Closed
This was referenced Sep 14, 2017
Support readonly type operator to allow correct definition of Object.freeze #10725
Closed
Eliminate non-generic functions #17428
Open
@msftgits Microsoft GitHub User (msftgits) removed the cla-already-signed label Sep 26, 2017
@microsoft Microsoft (microsoft) deleted a comment from Microsoft Contribution License Agreements (msftclas) Sep 27, 2017
@pelotom

Tom Crockett (pelotom) commented Oct 19, 2017

Copy link
Copy Markdown

This looks great! I am constantly annoyed by the fact that TypeScript doesn't always infer the most specific type. Is anyone taking a look at this PR?

bcherny reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@JohnNilsson

John Nilsson (JohnNilsson) commented Oct 22, 2017

Copy link
Copy Markdown

I came here looking for a "noImplicitWidening" option to disable all, well, implicit type widening. Is this PR an implementation of that?

franklixuefei reacted with thumbs up emoji franklixuefei reacted with heart emoji
All reactions
  • 👍 1 reaction
  • ❤️ 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@KiaraGrouwstra

kiara (KiaraGrouwstra) commented Oct 22, 2017

Copy link
Copy Markdown
Contributor Author

John Nilsson (@JohnNilsson): pretty much.

The exception here is the right-hand side of let declarations, intentionally left to widen expressions such as to offer users some control.
This means let arr = []; can still be used with mutating methods, unlike const arr = [];.

(Explicit annotations like const arr: number[] = []; work too -- just more effort.)

piotrwitek and andnp reacted with thumbs up emoji
All reactions
  • 👍 2 reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@pelotom

Tom Crockett (pelotom) commented Oct 22, 2017

Copy link
Copy Markdown

John Nilsson (@JohnNilsson) same, I just want a flag to disable type widening everywhere. If I ever need to widen a tuple to an array I’m fine with doing that via a manual cast.

JohnNilsson, piotrwitek, tvald, SlurpTheo, bykbtzr, ibezkrovnyi, and haltcase reacted with thumbs up emoji
All reactions
  • 👍 7 reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@KiaraGrouwstra kiara (KiaraGrouwstra) mentioned this pull request Oct 25, 2017
Design Meeting Notes, 10/13/2017 #19169
Closed
@gcnew Marin Marinov (gcnew) mentioned this pull request Nov 22, 2017
Suggestion: a way to disable type widening in object literals #20195
Closed
@pelotom Tom Crockett (pelotom) mentioned this pull request Apr 12, 2018
[typescript] withStyles() complains unless using as mui/material-ui#10995
Closed
1 task done
@KiaraGrouwstra kiara (KiaraGrouwstra) mentioned this pull request Apr 22, 2018
Subtraction types #4183
Closed
@bcherny

Boris Cherny (bcherny) commented Apr 22, 2018

Copy link
Copy Markdown

Any updates on this? Is this PR blocked on something, or can we make a yes/no call on whether this can make it in?

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@mhegazy

Mohamed Hegazy (mhegazy) commented Apr 23, 2018

Copy link
Copy Markdown
Contributor

I do not think this is a direction we are willing to peruse at the time being.

First, we really do not want flags that change type system behavior since that effectively creates dialects of the language; the ones we have allowed in the past fall into one of two categories, 1. temporary patches for breaking changes behavior (e.g. --noStrictGenericChecks) and 2. stricter behaviors that we think users should move to, but cost of migration is high (e.g. --strictNullChecks). The common part here is there is an eventual state, we believe, where the patching flags are not uses and the strict options are turned on; and that is our recommendation for new projects, and what we advertise in tsc --init.
This feature falls in neither of these two categories, so the flag is not something we would want to take.

Second, overloading the meaning of const is not something we are crazy about. we have done something similar with function parameters, but do not think we want to sanction this meaning of const declarations.

Sorry for the delay.

KiaraGrouwstra, s-ve, and anurbol reacted with thumbs up emoji KiaraGrouwstra reacted with confused emoji
All reactions
  • 👍 3 reactions
  • 😕 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@mhegazy Mohamed Hegazy (mhegazy) closed this Apr 23, 2018
@pelotom

Tom Crockett (pelotom) commented Apr 23, 2018

Copy link
Copy Markdown

Mohamed Hegazy (@mhegazy) is the TypeScript team thinking about how to solve the usability issues around type widening? In my mind and I think a lot of others it is the biggest wart on the language currently.

KiaraGrouwstra and alvis reacted with thumbs up emoji
All reactions
  • 👍 2 reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@mhegazy

Mohamed Hegazy (mhegazy) commented Apr 23, 2018

Copy link
Copy Markdown
Contributor

is the TypeScript team thinking about how to solve the usability issues around type widening?

Yes, and we continue to think of ways to alleviate these pains. One thing to note is it took us a few iterations to get where we are now. and there are many scenarios on both sides of any change at this point that would break. we try to balance all these as we introduce changes.

KiaraGrouwstra reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@KiaraGrouwstra

kiara (KiaraGrouwstra) commented Apr 23, 2018

Copy link
Copy Markdown
Contributor Author

Thanks for the response.

fwiw, I used the flag figuring this fell under # 2 -- stricter, (imo) preferable, some (imo minor) migration. I'd prefer not needing flags as well.

The semantic overloading of const I'll admit may be controversial.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@microsoft Microsoft (microsoft) locked and limited conversation to collaborators Jul 31, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

No reviews

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

8 participants

@KiaraGrouwstra @dead-claudia @pelotom @JohnNilsson @bcherny @mhegazy @msftclas @msftgits
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.