TypeScript icon indicating copy to clipboard operation
TypeScript copied to clipboard

Accessors should be allowed to be optional

Open justinfagnani opened this issue 2 years ago β€’ 7 comments

Suggestion

Right now accessors are barred from being declared optional. This is a DX regression for decorator users.

πŸ” Search Terms

accessor optional

βœ… Viability Checklist

My suggestion meets these guidelines:

  • [x] This wouldn't be a breaking change in existing TypeScript/JavaScript code
  • [x] This wouldn't change the runtime behavior of existing JavaScript code
  • [x] This could be implemented without emitting different JS based on the types of the expressions
  • [x] This isn't a runtime feature (e.g. library functionality, non-ECMAScript syntax with JavaScript output, new syntax sugar for JS, etc.)
  • [x] This feature would agree with the rest of TypeScript's Design Goals.

⭐ Suggestion

Allow accessors to be declared options;

πŸ“ƒ Motivating Example

Decorating a class field should require as few changes over a non-decorated field as necessary. Disallowing accessors from being optional adds an additional change.

Take a plain class:

class A {
  foo?: number;
}

Where you then want to make foo reactive with some library that vends decorators. This would be the change I would expect to make:

import {reactive, Base} from 'some-reactivity-lib';

class A extends Base {
  @reactive
  accessor foo?: number;
}

But 5.0 requires:

import {reactive, Base} from 'some-reactivity-lib';

class A extends Base {
  @reactive
  accessor foo: number | undefined;
}

I don't see a semantic problem with optional accessors. They behave very similar to optional fields (with standard field semantics). With useDefineForClassFields and this example:

class C {
  accessor foo?: number;
  
  bar?: number;
}

both foo and bar would exist on the runtime object even though they're optional, ie 'foo' in c === 'bar' in c.

πŸ’» Use Cases

More ergonomic decorator usage.

justinfagnani avatar May 13 '23 18:05 justinfagnani

Are you talking about accessors in general? If so then this duplicates #14417.

Are you only talking about auto accessors? If so then you might want to edit to use that term specifically. Of course it's still possible they'd consider this subsumed by #14417.

jcalz avatar May 13 '23 20:05 jcalz

See also https://github.com/microsoft/TypeScript/pull/16344, a PR which sought to implement this but seems to have fallen through (it’s now closed).

fatcerberus avatar May 13 '23 23:05 fatcerberus

@rbuckton Any thoughts?

RyanCavanaugh avatar May 15 '23 20:05 RyanCavanaugh

We just finished porting Lit's decorators to the new spec, and started porting some consumers via a code mod. The lack of optional auto accessors is standing out as a DX wart. Any chance this could be considered @rbuckton?

justinfagnani avatar Sep 05 '23 20:09 justinfagnani

accessor indicates a runtime transformation of a field to a get/set pair, and we don't currently allow get or set to be optional. Optionality is intended to indicate cases where the field itself may not exist, i.e., "x" in obj fails, which is made more apparent when you compile with --exactOptionalPropertyTypes. If you want accuracy (and portability with --exactOptionalPropertyTypes enabled), what you really want is exactly what is shown in the 5.0 example above, where you define the property as accessor x: Foo | undefined.

If you are already porting via a code mod, is there a reason that code mod cannot also drop the optionality and add | undefined?

Field optionality existed long before the native fields proposal, when it was completely feasible to declare an optional field and never actually define it (thus "x" in obj would return false). While --useDefineForClassFields means the field will always exist, it doesn't really match what ? was intended to mean. If we were being more strict, we might have disallowed ? on class fields. Then again, we allow ? on methods that are guaranteed to exist, and have for some time, despite that being primarily a feature for interface. I'm not strongly opposed to making allowing this, but it seems like a step in the wrong direction.

rbuckton avatar Sep 06 '23 01:09 rbuckton

If you are already porting via a code mod, is there a reason that code mod cannot also drop the optionality and add | undefined?

The code mod can, but not everyone's going to use the code mode, and new code won't. We have 10s to 100s of thousands of users.

For newly written code, the users still have to write:

@property() accessor foo: string | undefined;

when previously they were writing:

@property() foo?: string;

Every additional required keyword or boilerplate adds up

it doesn't really match what ? was intended to mean

I'm not sure how other developers think of ?, but I have always intuitively thought of it as part of the type signature only, like shorthand for | undefined. If that's how it behaves now for class fields and methods, then it seems entirely consistent to me to make it work that way for auto-accessors, and instead of a step in the wrong direction, it's a step towards consistency.

justinfagnani avatar Sep 06 '23 17:09 justinfagnani

As I port more decorator usage to standard decorators, the extra cost and keyword clutter of accessor foo: string | undefined vs accessor foo?: string is still very annoying.

When most people conceptually just want to write:

foo?: string;

but they need to use a decorator for reactivity, so they must do:

@reactive() foo?: string;

But decorators only work with auto-accessors, so they must do:

@reactive() accessor foo?: string;

But accessors aren't allowed to be optional, so they finally get to:

@reactive() accessor foo: string | undefined;

That's just a lot of extra boilerplate. The kind that honestly leads some people to JS forks.

Anything to reduce the sheer number of characters here would help, and optional accessors is basically the one thing within TypeScript's control.

justinfagnani avatar Oct 28 '24 17:10 justinfagnani