Fix #1809, introduce non primitive object type #12501

HerringtonDarkholme · 2016-11-25T16:49:56Z

Fixes #1809. I found object primitive type is quite confusing. Because what it means is that other primitive types cannot be assigned to. So I change the name to non primitive.

non primitive type is quite useful for ECMA API like Object.create, Object.setPrototype and Proxy.
It is also useful for userland libraries such as immutable and backbone.

DanielRosenwasser · 2016-11-25T17:09:52Z

src/compiler/checker.ts

@@ -1662,6 +1663,13 @@ namespace ts {
            return type;
        }

+        function createNonPrimitiveType(): ResolvedType {
+            const type = setStructuredTypeMembers(


@ahejlsberg can judge better, but I don't think you want to set any structured type members - I think you want to just need to add a branch in getApparentType for object.

DanielRosenwasser · 2016-11-25T17:11:15Z

src/compiler/checker.ts

@@ -3042,6 +3053,10 @@ namespace ts {
            return type && (type.flags & TypeFlags.Never) !== 0;
        }

+        function isTypeNonPrimitive(type: Type) {


This is only used in one place right now, so consider just inlining.

DanielRosenwasser · 2016-11-25T17:28:49Z

@HerringtonDarkholme thanks for the PR! As a heads up, we'll probably have to discuss this more at a design meeting, but I think we can iterate on it for a bit.

I think that a change like this would need the following tests:

Narrowing from object
- to an object type (e.g. instanceof, type predicates)
- to a primitive (e.g. typeof foo === "number", type predicates)
Narrowing from object | null | undefined (under strictNullChecks)
- With == null
- With === null
- With typeof foo === "object"
- With typeof foo === "undefined"
Assigning (under strictNullChecks)
- A null to something of type object.
- A undefined to something of type object.
- Something of type Number, String, or Boolean to something of type object.
Trying to access members (under noImplicitAny and strictNullChecks)
- that exist on Object.prototype (e.g. toString, hasOwnProperty)
- that don't exist on Object.prototype (e.g. abcdef)

Most of these should all run under noImplicitAny and strictNullChecks personally.

HerringtonDarkholme · 2016-11-25T17:45:38Z

Thanks for your quick response and kind suggestion!

I'm quite unsure about the behavior of narrowing behavior.

For example, an empty class like class A {}, should instanceof narrow object to A? Then other primitives will be assignable to object in that branch. Also, should typeof foo === "number" yield never type? I think that will be reasonable.(But I didn't add that test since #1809 does not specify it and TS has changed a lot since then.)

Thanks for the review! I will research other issues!

DanielRosenwasser · 2016-11-25T18:18:15Z

For example, an empty class like class A {}, should instanceof narrow object to A? Then other primitives will be assignable to object in that branch.

If I'm not mistaken TypeScript uses the declared type (i.e. the original type) when getting the left side of an assignment. In those cases, the type should still be object, which should in turn should save you.from that concern, but it'd definitely be very helpful to have a test for this.

alitaheri · 2016-11-25T18:51:35Z

This is a great feature, specially when dealing with generic constraints. Can it fully cover the bellow case?

interface Proxy<T extends object> {
  // ...
}

// fails:
const a: Proxy<number> = { ... };
const c: Proxy<null> = { ... };
const c: Proxy<undefined> = { ... };

// won't fail:

interface Blah {
  foo: number;
}

const b: Proxy<Blah> = { ... };

HerringtonDarkholme · 2016-11-26T05:14:35Z

Thanks for all your valuable suggestion!
@alitaheri Proxy<null> will only fail under strictNullChecks. I think a test is needed here.

dead-claudia · 2016-12-13T14:51:08Z

Question: what about functions? They're also reference types that can be Object.created and Object.assigned to.

Edit: I don't know what I'm talking about... they implicitly subtype Function.

sandersn

I like the semantics but I agree with @DanielRosenwasser that (1) we need to have the whole team discuss at a design meeting (2) nonPrimitiveType should be a simple marker type, more like stringType, numberType etc and then add a branch in getApparentType. IntrinsicType with TypeFlags.Object might even work.

sandersn · 2016-12-15T23:37:27Z

tests/baselines/reference/nonPriimitiveInFunction.errors.txt

@@ -0,0 +1,31 @@
+tests/cases/conformance/types/nonPrimitive/nonPriimitiveInFunction.ts(12,12): error TS2345: Argument of type 'boolean' is not assignable to parameter of type 'object'.


typo in filename: Priimitive

sandersn

A few comments on the tests

sandersn · 2016-12-15T23:43:38Z

tests/baselines/reference/nonPrimitiveAssignError.errors.txt

+    var x = {};
+    var y = {foo: "bar"};
+    var a: object;
+    x = a;


what about a = x and a = y? I think neither should have errors.

sandersn · 2016-12-15T23:45:29Z

tests/baselines/reference/nonPrimitiveInGeneric.errors.txt

+                 ~~~~~~
+!!! error TS2344: Type 'number' does not satisfy the constraint 'object'.
+    var y: Proxy<null>; // ok
+    var z: Proxy<undefined> ; // ok


line 51 and 52 are only ok because strictNullChecks is not turned on, right?

Yes, and there are corresponding tests in nonPrimitiveStrictNull.ts which produces errors.

sandersn · 2016-12-15T23:52:04Z

tests/baselines/reference/nonPrimitiveInGeneric.errors.txt

+
+
+==== tests/cases/conformance/types/nonPrimitive/nonPrimitiveInGeneric.ts (7 errors) ====
+    function generic<T>(t: T) {}


except for null and undefined, I don't think these tests are that valuable, since they are just testing assignability again

HerringtonDarkholme · 2016-12-16T03:36:17Z

@sandersn Pardon my question.
I have tried to replace nonPrimitive type by createIntrinsicType(TypeFlags.Object, "object"). But I get more errors in objectTypeRelatedTo, isFunctionObjectType and etc, where nonPrimitiveType is used as target and is directly compared rather than its apparent type. Currently I create nonPrimitiveType by reusing TypeFlags.Object so that all object type checking logic is reused.

I wonder if nonPrimitiveType can be labelled as intrinsic. One intrinsic type needs to use one bit flag like Number or String which I think is too expensive for non primitive checking.

Maybe I can change the TypeFlags? Or patching more functions?

sandersn · 2016-12-16T18:00:17Z

Yeah, I talked to @ahejlsberg and he said that object needs a new TypeFlags entry of its own. This probably means that you'll have to handle it specially in more places. I think, for most of these places, it's a good idea to have object-specific code in place, though.

HerringtonDarkholme · 2016-12-17T05:07:04Z

Thanks for the clarification! I'm quite hesitant about adding TypeFlags since TypeScript will have more features like variadic type parameter and exact type, which all need TypeFlag entries.

I think I will wait for your whole team discussion.

HerringtonDarkholme · 2016-12-19T05:49:18Z

I have updated the implementation to use intrinsic type. One thing I'm not sure is whether intrinsic object should be counted as StructuredType. It seems StructuredType flag is used in narrowing type, so adding nonPrimitive to StructuredType enables narrowing object by instanceof.

But if intrinsic type is used in resolveStrucutredTypeMembers, intrinsic type will be returned as resolvedType while it isn't. (I didn't managed to create a test case to cover that.)

I'm very grateful to you for all your review! 😃

Update:
TSLint breaks CI. Awaiting #13006

sandersn · 2016-12-22T00:12:50Z

I don't think you should re-use StructuredType either -- it's a generalisation of Object that includes any type that will eventually have members but may not yet. I think the object intrinsic should stay separate.

HerringtonDarkholme · 2016-12-22T02:17:34Z

Thanks for your advice! I have changed nonPrimitive to Narrowable from StructuredType.
Also rebased.

sandersn · 2017-01-06T17:52:05Z

Thanks so much @HerringtonDarkholme!

(And sorry for the delay from the holidays.)

glazar · 2017-02-21T11:21:18Z

One common task is to take an existing type and make each of its properties entirely optional.
Thus we have support for Partial since 2.1.

The issue with using Partial is that any usages of Partial also accepts primitives, even though the intent is that primitives be excluded.

Is this new object type, added with 2.2, used by default when using Partial construct ?

It would make sense to make use of it, since it would solve the issue of usages of Partial also accepting primitives.

sandersn · 2017-02-21T16:43:23Z

@glazar That seems reasonable. Can you create an issue for that so that we can track it separately? The PR should be simple.

gcnew · 2017-02-21T16:48:12Z

If that were the case, wouldn't it break the currently used "deep partial" implementations? The same applies to Readonly, I think.

sandersn · 2017-02-21T18:13:46Z

@gcnew can you give an example of a deep partial type? The Partial<T> in lib.d.ts looks shallow to me.

As I recall, the basic problem with a recursive partial type is that we have no way to stop the recursion right now, so it does in fact recur right though primitives, chopping them up into length, charAt, charCodeAt, etc. Eventually I suspect that mapped types will add conditionals so you can stop the recursion upon reaching primitives.

glazar · 2017-02-22T08:48:20Z

@sandersn @gcnew
I have created a dedicated issue for the situation I mentioned in my previous comment - #14224

gcnew · 2017-02-22T14:35:44Z

@sandersn I don't have anything specific in mind. I think I've seen people asking question and submitting hacky code that might be affected. On a more serious note, will it work correctly if the target type has type parameters?

msftclas added the cla-already-signed label Nov 25, 2016

DanielRosenwasser reviewed Nov 25, 2016

View reviewed changes

dead-claudia mentioned this pull request Dec 13, 2016

Enhance Object.assign static method signature #12110

Closed

This was referenced Dec 14, 2016

TS not indicating type missmatches #12914

Closed

Two types should only be compatible if their members are compatible - even if all these members are optional #12904

Closed

sandersn reviewed Dec 15, 2016

View reviewed changes

sandersn requested changes Dec 15, 2016

View reviewed changes

DanielRosenwasser mentioned this pull request Dec 17, 2016

Design Meeting Notes, 12/16/2016 #12997

Closed

HerringtonDarkholme added 10 commits December 22, 2016 10:04

Fix microsoft#1809, introduce non primitive type

ebe2fdb

enable parsing

db9dd38

fix error reporting

ce4c95f

add tests for non primitive type

b8648fa

address code review feedback

2fb51e7

accept new baselines

c19221c

add tests for generic type argument under strict null checks

186ce38

address CR feedback

8b411ef

update implementation to use intrinsic type

66069fc

classify nonPrimitive type as narrowable rather than structured

8993877

HerringtonDarkholme force-pushed the non-primitive branch from 93b6354 to 8993877 Compare December 22, 2016 02:07

sandersn approved these changes Jan 6, 2017

View reviewed changes

sandersn merged commit e9e7fce into microsoft:master Jan 6, 2017

HerringtonDarkholme deleted the non-primitive branch January 8, 2017 10:11

HerringtonDarkholme mentioned this pull request Jan 8, 2017

update declaration file to use intrinsic object type #13350

Merged

saschanaz mentioned this pull request Sep 16, 2017

Less restrictive object type #18228

Closed

DanielRosenwasser mentioned this pull request Nov 28, 2017

Spec needs to document the non-primitive 'object' type. #20313

Closed

microsoft locked and limited conversation to collaborators Jun 19, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix #1809, introduce non primitive object type #12501

Fix #1809, introduce non primitive object type #12501

HerringtonDarkholme commented Nov 25, 2016 •

edited by DanielRosenwasser

DanielRosenwasser Nov 25, 2016

DanielRosenwasser Nov 25, 2016

DanielRosenwasser commented Nov 25, 2016

HerringtonDarkholme commented Nov 25, 2016

DanielRosenwasser commented Nov 25, 2016 •

edited

alitaheri commented Nov 25, 2016

HerringtonDarkholme commented Nov 26, 2016

dead-claudia commented Dec 13, 2016 •

edited

sandersn left a comment

sandersn Dec 15, 2016

HerringtonDarkholme Dec 16, 2016

sandersn left a comment

sandersn Dec 15, 2016

HerringtonDarkholme Dec 16, 2016

sandersn Dec 15, 2016

HerringtonDarkholme Dec 16, 2016 •

edited

sandersn Dec 15, 2016

HerringtonDarkholme commented Dec 16, 2016 •

edited

sandersn commented Dec 16, 2016

HerringtonDarkholme commented Dec 17, 2016

HerringtonDarkholme commented Dec 19, 2016 •

edited

sandersn commented Dec 22, 2016

HerringtonDarkholme commented Dec 22, 2016

sandersn commented Jan 6, 2017

glazar commented Feb 21, 2017

sandersn commented Feb 21, 2017

gcnew commented Feb 21, 2017

sandersn commented Feb 21, 2017

glazar commented Feb 22, 2017

gcnew commented Feb 22, 2017 •

edited

		@@ -0,0 +1,31 @@
		tests/cases/conformance/types/nonPrimitive/nonPriimitiveInFunction.ts(12,12): error TS2345: Argument of type 'boolean' is not assignable to parameter of type 'object'.



		==== tests/cases/conformance/types/nonPrimitive/nonPrimitiveInGeneric.ts (7 errors) ====
		function generic<T>(t: T) {}

Fix #1809, introduce non primitive object type #12501

Fix #1809, introduce non primitive object type #12501

Conversation

HerringtonDarkholme commented Nov 25, 2016 • edited by DanielRosenwasser

Choose a reason for hiding this comment

Choose a reason for hiding this comment

DanielRosenwasser commented Nov 25, 2016

HerringtonDarkholme commented Nov 25, 2016

DanielRosenwasser commented Nov 25, 2016 • edited

alitaheri commented Nov 25, 2016

HerringtonDarkholme commented Nov 26, 2016

dead-claudia commented Dec 13, 2016 • edited

sandersn left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sandersn left a 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

HerringtonDarkholme Dec 16, 2016 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

HerringtonDarkholme commented Dec 16, 2016 • edited

sandersn commented Dec 16, 2016

HerringtonDarkholme commented Dec 17, 2016

HerringtonDarkholme commented Dec 19, 2016 • edited

sandersn commented Dec 22, 2016

HerringtonDarkholme commented Dec 22, 2016

sandersn commented Jan 6, 2017

glazar commented Feb 21, 2017

sandersn commented Feb 21, 2017

gcnew commented Feb 21, 2017

sandersn commented Feb 21, 2017

glazar commented Feb 22, 2017

gcnew commented Feb 22, 2017 • edited

HerringtonDarkholme commented Nov 25, 2016 •

edited by DanielRosenwasser

DanielRosenwasser commented Nov 25, 2016 •

edited

dead-claudia commented Dec 13, 2016 •

edited

HerringtonDarkholme Dec 16, 2016 •

edited

HerringtonDarkholme commented Dec 16, 2016 •

edited

HerringtonDarkholme commented Dec 19, 2016 •

edited

gcnew commented Feb 22, 2017 •

edited