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

Variant accessors #42425

Merged
merged 20 commits into from
Mar 27, 2021
Merged

Conversation

RyanCavanaugh
Copy link
Member

@RyanCavanaugh RyanCavanaugh commented Jan 20, 2021

Getter / Setter Variation

Implements #2845 and #2521

Overview

This implements two related features:

Differing Types

Property getters and setters may now differ in their types:

const thing = {
    _size: 0,
    get size(): number {
        return this._size;
    },
    set size(value: string | number) {
        this._size = typeof value === 'string' ? parseInt(value) : value;
    }
}

// OK
thing.size = "100ish";
// OK
console.log(thing.size.toFixed(2));

Restrictions

As a conservative implementation, a few restrictions are in place.

First, the type of the getter must be assignable to the type of the setter. In other words, the assignment

obj.x = obj.x;

must be legal assuming obj.x is writable at all.

This restriction closes off a certain set of use cases for properties that start out "uninitialized" (usually null/undefined) but then only want "initalizing" assignments to occur. We'll continue to examine these to see if those use cases are prevelant enough to warrant opening this up more.

These prevent novel unsoundness from occurring and makes this feature unimpactful from a type relational perspective, which limits its risk.

Differing Visibility

In classes, set accessors may now be less visible than their corresponding get accessor. It looks like this:

class MyClass {
    get size(): number {
        // TODO: implement
        return 0;
    }
    protected set size(value: number) {
        // TODO: implement
    }

    grow() {
        // OK
        this.size++;
    }
}

let c = new MyClass();
// Error, can't write to 'size' outside of 'MyClass'
c.size = 10;
// OK
console.log(c.size);

Type Relationship Effects

TL;DR: there are none

TypeScript is already covariant when relating properties of types. The unsoundness of this is straightforward to demonstrate during aliased writes:

type Alpha = { x: string | number };
type Beta = { x: string };

function mutate(ref: Alpha) {
    ref.x = 0;
}
const p: Beta = { x: "hello" };
mutate(p);
// Prints 'number' on a binding typed as 'string', oops.
console.log(typeof p.x);

TypeScript also already ignores the readonly modifier when relating types, so the protected or private state of a setter does follows the same pattern.

Caveats

TL;DR: The type system remains entirely covariant in other operations! This has some effects that may not be immediately apparent.

Types with variant getters/setters are effectively reduced to their get side when put through mapped types:

// Setter types are intentionally not preserved
// through mapped types or other transforms; they
// are specific to their originating declaration
declare const CoercingThing: {
    get size(): number;
    set size(v: number | string);
}
type PCT = Partial<typeof CoercingThing>;
declare const pct: PCT;
// Not OK
pct.size = "hello";

This is fairly straightforward to reason about -- a mapped type usually represents a "copy with transform" operation, e.g.

(psuedocode: some type operation f applied to an object type obj)
result = { }
for k in keyof obj
  result[k] = f(obj[k]);
return result

In other words, for an arbitrary mapped type, we have no idea how its actual value is produced, and the only sound assumption is that this type doesn't copy over any coercing semantics from its setters.

The same applies to lookup types -- the type T[K] still means *the read type of K on T. A free setter function can't be used to indirectly access the coercing side of the setter:

declare const CoercingThing: {
    get size(): number;
    set size(v: number | string);
}
function setter<T, K extends keyof T>(obj: T, key: K, value: T[K]) {
    obj[key] = value;
}
// Not OK, even though CoercingThing.size = "100" is OK
setter(CoercingThing, "size", "100");

@typescript-bot typescript-bot added Author: Team For Uncommitted Bug PR for untriaged, rejected, closed or missing bug labels Jan 20, 2021
@RyanCavanaugh
Copy link
Member Author

@typescript-bot pack this

@typescript-bot
Copy link
Collaborator

typescript-bot commented Jan 20, 2021

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

@typescript-bot
Copy link
Collaborator

typescript-bot commented Jan 20, 2021

Hey @RyanCavanaugh, 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/93799/artifacts?artifactName=tgz&fileId=3EC3DF89823D37BAFB0102D05E0C5878E1CC64D470DA739FA701667C9C8CEEC602&fileName=/typescript-4.2.0-insiders.20210120.tgz"
    }
}

and then running npm install.


There is also a playground for this build and an npm module you can use via "typescript": "npm:@typescript-deploys/pr-build@4.2.0-pr-42425-2".;

@RyanCavanaugh
Copy link
Member Author

@typescript-bot pack this

@typescript-bot
Copy link
Collaborator

typescript-bot commented Jan 29, 2021

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

@typescript-bot
Copy link
Collaborator

typescript-bot commented Jan 29, 2021

Hey @RyanCavanaugh, 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/94491/artifacts?artifactName=tgz&fileId=5D2DE7C1D3D1A47BE543A3A8978BBC0D64E3E7103505294FB8EDC8B577A31AA302&fileName=/typescript-4.2.0-insiders.20210129.tgz"
    }
}

and then running npm install.


There is also a playground for this build and an npm module you can use via "typescript": "npm:@typescript-deploys/pr-build@4.2.0-pr-42425-5".;

@RyanCavanaugh RyanCavanaugh changed the title [Draft] Variant accessors Variant accessors Feb 3, 2021
@RyanCavanaugh RyanCavanaugh marked this pull request as ready for review February 3, 2021 20:31
@ExE-Boss
Copy link
Contributor

ExE-Boss commented Feb 5, 2021

This probably also fixes #32821

@trusktr
Copy link
Contributor

trusktr commented Feb 17, 2021

What milestone will this go into?

trusktr added a commit to lume/lume that referenced this pull request Feb 17, 2021
…ariant accessor types in TypeScript

This make accessors have the correct types to represent all possible values that they can be set to, but the getters are funky because to use them it requires type casting. As a temporary workaround, `get*()` methods have been added (f.e. `element.getPosition()` returns the same as `element.position` but with the value casted to the underlying object type.

This does not impact JavaScript users. They can continue to do things like `element.position.x = 123`. But TypeScript users can't do that. They would have to write `;(element.position as XYZNumberValues).x = 123` which is cumbersome, but the temporary getter methods allow TypeScript users to write `element.getPosition().x = 123` instead. TypeScript users can still set values as usual: `this.position = [1, 2, 3]` just like JavaScript users.

The upcoming changes in microsoft/TypeScript#42425 will allow us to update the getter types so TypeScript users can write `element.position.x = 123`.
@RyanCavanaugh
Copy link
Member Author

@typescript-bot pack this

@typescript-bot
Copy link
Collaborator

typescript-bot commented Mar 25, 2021

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

@typescript-bot
Copy link
Collaborator

typescript-bot commented Mar 25, 2021

Hey @RyanCavanaugh, 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/99188/artifacts?artifactName=tgz&fileId=8A4F5CF4310E72AE3F3599DB677A807009BC386229856DAD4B06C74D77071B7702&fileName=/typescript-4.3.0-insiders.20210325.tgz"
    }
}

and then running npm install.


There is also a playground for this build and an npm module you can use via "typescript": "npm:@typescript-deploys/pr-build@4.3.0-pr-42425-10".;

# Conflicts:
#	src/compiler/checker.ts
#	src/compiler/diagnosticMessages.json
#	tests/baselines/reference/privateNamesAndGenericClasses-2.errors.txt
@RyanCavanaugh
Copy link
Member Author

@ahejlsberg thanks, addressed at #43405

@RyanCavanaugh RyanCavanaugh deleted the variantAccessors branch March 29, 2021 16:02
lgarron added a commit to cubing/cubing.js that referenced this pull request Apr 3, 2021
This will depend on microsoft/TypeScript#42425 landing in stable TypeScript so we can update the types to reflect it.
@kevinclarkadstech
Copy link

@typescript-bot pack this

@pikax
Copy link

pikax commented Apr 26, 2021

What's the syntax (if supported) for generic types?

Simplified Example:

type MarkAsString<T extends Record<string, any>> = {
    [K in keyof T]: T extends number ? T[K] /* How to set add a string|number set here? */ : T[K]
}

declare const a: MarkAsString<{ a: boolean, b: number, c: string }>
a.b = '42'; // I want to have this avalible

a.c = '112'

// @ts-expect-error
a.a = '2'

Actual use case

In Vue3 there's Ref<T> (simplified as { value: T}) and Reactive<UnwrappedRef<T>> (unwraps {value: T} to T, recursive)
Basically:

const r = ref({ a: 1} )

r.value.a = 1

const a = reactive(r)
a.a // 1

On a reactive/ref object the set in typescript must be UnwrappedRef<T> which is not true, because when you assign to a reactive it will unwrap the value:

const r = ref({a:1});
const a = ref({
  r
}) // results in `{ r: { a: 1 } }` 

a.r = r; // typescript error  because `r` is `Ref<T>` instead of `UnwrappedRef<T>`, but it works at runtime.

@thw0rted
Copy link

thw0rted commented Apr 26, 2021

I think what you're actually looking for is how to use the new feature with index signatures. My first guess would have been something like

interface ConvertingRecord {
  get [k: string](): string;
  set [k: string](val: string | number);
}

but that doesn't work on the latest Playground for this build. Maybe it's not currently possible?

@RyanCavanaugh
Copy link
Member Author

Yeah, that's a totally separate and additional feature.

@pikax
Copy link

pikax commented Apr 26, 2021

Yeah, that's a totally separate and additional feature.

Cool, should I open an issue?

@RyanCavanaugh
Copy link
Member Author

Sure

@V1raNi
Copy link

V1raNi commented Aug 4, 2021

This is very cool, is there any information on which version will have this change?

@yume-chan
Copy link
Contributor

@ws93
Copy link

ws93 commented Aug 7, 2021

The different type for setter doesn't work for Union type as well. Is this expected? Example can be found here

@thw0rted
Copy link

thw0rted commented Aug 9, 2021

@ws93 I didn't see an issue for that yet so I went ahead and opened #45376 .

@V1raNi
Copy link

V1raNi commented Aug 11, 2021

@V1raNi This has been shipped in 4.3 https://www.typescriptlang.org/docs/handbook/release-notes/typescript-4-3.html#separate-write-types-on-properties

Oh, thank you very much. It's just I was interested in different visibility and couldn't find it in the text, and the docs seem to be outdated a bit regarding this part.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Author: Team For Uncommitted Bug PR for untriaged, rejected, closed or missing bug
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.