`typeof x` now has a string literal union type #13791

sandersn · 2017-01-31T18:41:13Z

For implementations that disobey the advice of the standard, it is easy to work around the type, although narrowing no longer works:

if (typeof x === "ActiveXObject" as string) {
  // no narrowing here
} else {
  // or here
}

Fixes #9506
Fixes #13771 (by making the incorrect narrowing an error)

Previously, it was just a string

Test that `typeof x === 'random' as string`: 1. Does not issue an error. 2. Does not narrow.

vladima · 2017-01-31T22:02:00Z

src/compiler/checker.ts

@@ -14609,7 +14609,7 @@ namespace ts {

        function checkTypeOfExpression(node: TypeOfExpression): Type {
            checkExpression(node.expression);
-            return stringType;
+            return getUnionType(arrayFromMap(typeofEQFacts.keys(), s => getLiteralTypeForText(TypeFlags.StringLiteral, s)));


can this union type be precomputed and cached to avoid doing the same work multiple times?

Yes. I wasn't sure where to put it though.

Take a look at the new commit; I added a new typeofType that is created when the checker is.

Note that it changes the print order of string literal unions slightly. I think this is because 'string', 'symbol', etc are added to a union very early in the creation of the checker.

This changes the print order of string literal unions slightly. I think this is because 'string', 'symbol', etc are added to a union very early on in the creation of the checker.

vladima · 2017-02-01T19:32:36Z

src/compiler/core.ts

@@ -895,6 +895,14 @@ namespace ts {
        return result;
    }

+    export function arrayFromMap<T, U>(iterator: Iterator<T>, f: (value: T) => U) {


I'd say this is iteratorToArray to arrayFromIterator since there is nothing map specific in the signature

I meant map in the sense of the map function not the Map type. Maybe mapIterator? mapIteratorToArray? Something else?

maybe convertToArray?

I like that one.

vladima · 2017-02-01T22:37:59Z

tests/baselines/reference/typeGuardOfFormTypeOfOther.errors.txt

+    //  - when true, removes the primitive types string, number, and boolean from the type of x, or
+    //  - when false, has no effect on the type of x.
+
+    if (typeof strOrC === "Object") {


is intent here to make C object in the true branch and string - in false one?

I believe so. Javascript doesn't work that way, of course, so that's why this PR makes it an error.

Note that since 2.0 it has worked this way. Previously, the false branch had no effect on the type of x.

treshugart · 2017-02-14T21:45:57Z

@mhegazy Hey, got linked to this from the interwebs. Was going to adopt TS in SkateJS but it looks like you're making a breaking change as part of a patch milestone?

mhegazy · 2017-02-14T21:47:41Z

Can you elaborate on how SkateJS would be broken by this change?

sandersn · 2017-02-14T21:47:57Z

2.2.0 is 2.2 RC; 2.2.1 is the release version of 2.2.

treshugart · 2017-02-15T01:50:22Z

Can you elaborate on how SkateJS would be broken by this change?

We're not using it yet, but just breaking Skate doesn't concern me. Possibly breaking every consumer on a minor version does. I could be wrong, but this appears to be a change between the 2.1.x stable, minor range to the 2.2.x stable, minor range, correct? I could have easily missed something here.

2.2.0 is 2.2 RC; 2.2.1 is the release version of 2.2.

I apologise for my ignorance, I just couldn't infer that 2.2.1 was the stable after the 2.2 rc. Thanks for clarifying :)

mhegazy · 2017-02-15T02:04:54Z

I see, thanks for the explanation.

We make all changes that could result in an error, if previously was not, as "breaking" changes. We promise no breaking changes in revisions, e.g. 2.2.1 -> 2.2.2. but not in minor version upgrades e.g. 2.1 -> 2.2. We recommend that you fix your package.json to major.moinor of TypeScript. Please see https://github.com/Microsoft/TypeScript/wiki/Breaking-Changes for the full list of breaking changes in the the past.

This change makes the type of typeof expressions to be 'string' | 'number' | 'boolean' | 'symbol' | 'undefined' | 'object' | 'function', which should cover all values you would get from one of the popular JS engines. The reason it is marked as breaking, is per the ES spec, an engine could generate a different value, so this is a "breaking change" if you happen to be running against an engine (that is not v8, chackra, spidermoneky, or Core).

BenSayers · 2017-04-06T06:34:16Z

What the reasoning behind not following semver? Npm docs recommend that modules follow server and will automatically allow minor bumps in a module by default. I'm concerned that not following this convention will create unnecessary friction for TypeScript consumers who will by default be opted in to having their builds broken whenever TypeScript releases a minor version.

sandersn · 2017-04-06T16:08:17Z

@BenSayers @treshugart the official semver issue is at #14116. You can read some of our reasoning and continue the discussion there.

sandersn added 3 commits January 31, 2017 10:28

typeof now has string literal union type

d77945f

Previously, it was just a string

Add typeof test case and update baselines

69e9bfe

Test that `typeof x === 'random' as string`: 1. Does not issue an error. 2. Does not narrow.

Add arrayFromMap utility function

01bc4ef

msftclas added the cla-not-required label Jan 31, 2017

vladima reviewed Jan 31, 2017

View reviewed changes

Create typeof's string literal union type once

2ce64f2

This changes the print order of string literal unions slightly. I think this is because 'string', 'symbol', etc are added to a union very early on in the creation of the checker.

vladima reviewed Feb 1, 2017

View reviewed changes

rename arrayFromMap -> convertToArray

d23f692

vladima reviewed Feb 1, 2017

View reviewed changes

Merge branch 'master' into typeof-string-literal-union-type

bcd1ef8

sandersn merged commit 363d914 into master Feb 7, 2017

sandersn deleted the typeof-string-literal-union-type branch February 7, 2017 21:19

ajafff mentioned this pull request Feb 7, 2017

Deprecate typeof-compare palantir/tslint#2187

Closed

mhegazy added the Breaking Change Would introduce errors in existing code label Feb 8, 2017

mhegazy added this to the TypeScript 2.2.1 milestone Feb 8, 2017

renovate-bot mentioned this pull request Feb 9, 2017

Update dependency typescript to version 2.1.6 singapore/lint-condo#237

Merged

fvoska mentioned this pull request Apr 9, 2018

Replace deprecated rules infinum/tslint-config#8

Merged

microsoft locked and limited conversation to collaborators Jun 19, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

`typeof x` now has a string literal union type #13791

`typeof x` now has a string literal union type #13791

sandersn commented Jan 31, 2017 •

edited

Loading

vladima Jan 31, 2017

sandersn Jan 31, 2017

sandersn Jan 31, 2017

vladima Feb 1, 2017

sandersn Feb 1, 2017

vladima Feb 1, 2017

sandersn Feb 1, 2017

vladima Feb 1, 2017

sandersn Feb 1, 2017

treshugart commented Feb 14, 2017

mhegazy commented Feb 14, 2017

sandersn commented Feb 14, 2017

treshugart commented Feb 15, 2017

mhegazy commented Feb 15, 2017

BenSayers commented Apr 6, 2017 •

edited

Loading

sandersn commented Apr 6, 2017

typeof x now has a string literal union type #13791

typeof x now has a string literal union type #13791

Conversation

sandersn commented Jan 31, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

treshugart commented Feb 14, 2017

mhegazy commented Feb 14, 2017

sandersn commented Feb 14, 2017

treshugart commented Feb 15, 2017

mhegazy commented Feb 15, 2017

BenSayers commented Apr 6, 2017 • edited Loading

sandersn commented Apr 6, 2017

`typeof x` now has a string literal union type #13791

`typeof x` now has a string literal union type #13791

sandersn commented Jan 31, 2017 •

edited

Loading

BenSayers commented Apr 6, 2017 •

edited

Loading