Update README with more details #354

pzuraq · 2021-01-22T01:57:46Z

Updates the README with a more detailed breakdown of the API, along with
a number of tweaks to the APIs including:

Adds the prop keyword and class element type
Adds the init keyword and class element type
Updates field semantics to not transform to getter/setter pairs by default
Adds the access property to the context object for private elements
Refactors the metadata API to be more intuitive and well defined

chriskrycho · 2021-01-22T03:54:14Z

README.md

-  @logged
-  m(arg) {
-    this.#x = arg;
+- Class _properties_, defined by applying the `prop` keyword to a class field. These have a getter and setter, unlike fields, which default to getting and setting the value on a private storage slot (equivalent to a private class field):


Slight ambiguity from the phrasing here: it currently reads as though fields default t o getting and setting on a private storage slot.

Suggested change

- Class _properties_, defined by applying the `prop` keyword to a class field. These have a getter and setter, unlike fields, which default to getting and setting the value on a private storage slot (equivalent to a private class field):

- Class _properties_, defined by applying the `prop` keyword to a class field. Unlike fields, these have a getter and setter, which default to getting and setting the value on a private storage slot (equivalent to a private class field):

chriskrycho · 2021-01-22T04:00:17Z

README.md

+  - `"class"`
+  - `"method"`
+  - `"init-method"`
+  - `"getter"`
+  - `"setter"`
+  - `"field"`
+  - `"prop"`


Given the discussion below, I'm led to wonder whether there's value in distinguishing private and public here as well—given that there are values you can count on being present or absent for public or private fields or methods respectively. 🤔

I see that this is covered by the isPrivate boolean value as well; just thinking about what the experience consuming it would be like. It may well be that narrowing down the thing being decorated by first checking kind and then checking isPrivate is totally reasonable.

private is an orthogonal property that can apply to almost all of these. I think it'd be a bit awkward if you were forced to parse the string, if we had, e.g., "public-method" vs "private-method", to figure out if it's a method at all.

Yeah, that makes sense. 👍🏼

chriskrycho · 2021-01-22T04:10:26Z

README.md

 class C {
-  #x = 1;
-  @init: bound method() { return this.#x; }
+  @validateString


Suggested change

@validateString

@validatePrivateString

README.md

ljharb · 2021-01-22T04:02:54Z

README.md

-  @logged
-  m(arg) {
-    this.#x = arg;
+- Class _properties_, defined by applying the `prop` keyword to a class field. These have a getter and setter, unlike fields, which default to getting and setting the value on a private storage slot (equivalent to a private class field):


“property” already has a meaning (a normal object property) - i think it’s important here to come up with a term that isn’t overloaded already. Class properties already exist: they’re static fields and methods.

I agree with this point and considered it when writing this, it's something I want to bring up in the upcoming meetings. If we do stick with the prop keyword, then this conflict may pop up in the future, especially in common usage among software devs. We should definitely consider alternatives, both for the name of the construct itself, and for the keyword, but there was broad consensus that the keyword was the best option suggested so far.

ljharb · 2021-01-22T04:05:10Z

README.md

+```ts
+type ClassMethodDecorator = (value: Function, context: {
+  kind: "method";
+  name?: string;


surely this could also be a Symbol?

Suggested change

name?: string;

name?: PropertyKey;

ljharb · 2021-01-22T04:06:39Z

README.md

+```ts
+type Decorator = (value: Input, context: {
+  kind: string;
+  name?: string;


Suggested change

name?: string;

name?: PropertyKey;

Agreed, or alternatively, string | symbol to make it more readable for people who might not know the jargon.

ljharb · 2021-01-22T04:09:34Z

README.md

+```ts
+type ClassGetterDecorator = (value: Function, context: {
+  kind: "getter";
+  name?: string;


Suggested change

name?: string;

name?: PropertyKey;

ljharb · 2021-01-22T04:22:45Z

README.md

+```ts
+type ClassDecorator = (value: Function, context: {
+  kind: "class";
+  name: string;


what gets passed here for an anonymous class, or one that gets an inferred name? I’d assume undefined is appropriate for the former (the empty string probably wouldn’t be). I haven’t tested it, but i think the latter means the name could also be a Symbol

ljharb · 2021-01-22T04:25:23Z

README.md

 ```

-### Limited access to private fields and methods
+Properties, unlike fields, define a getter and setter on the class prototype. This getter and setter default to getting and setting a value on a private slot. The above roughly desugars to:


what is the point of doing this? How is an undecorated prop any different than a public field? (i suppose it does a brand check, but is there a use case for that?)

The answer to your question about how an undecorated prop differs from a public field in semantics is below in this document. As for use cases: I could imagine using prop fields to faithfully emulate other cases in JS and the web platform where such patterns are used by built-in features. But really, what brings them over the bar for being "worth it" to add to the language is decorators, IMO.

I think @decorator prop field; makes a lot more sense than @prop: decorator field; since prop changes the shape of the class in a way that's unrelated to the application of any one particular decorator. (I guess init in this proposal has that same property, though.)

Right, the thinking is that prop fundamentally changes the semantics of a field, and then decorators layer naturally on top of the new semantics. While the semantics on their own have limited use cases, there are many use cases for decorators with these semantics. Given the other constraints around shape changes and static analyzability, this option keeps the application of decorators consistent while solving all of the use cases.

Separate from the name (which is imo a nonstarter), I don't think we should be adding fundamental field semantics in the absence of decorators for imagined use cases.

I think @prop: decorator and @init: decorator are indeed the same in this sense, and i'd prefer to see both of them remain exclusively attached to decorator syntax.

The difficulty comes with combining @prop: decorator and @decorator declarations then. Does applying one @prop: decorator then turn the field into a prop-field, and all other decorators now need to handle that prop-field? Likewise with init-methods.

This to me signals that the @prop: style modifier is fundamentally in the wrong location. It is not actually modifying the decorator, it is modifying the element itself, so it is correct to place the modifier on the element, regardless of whether it has utility on its own.

Another thing to note is that the current location would compose nicely with the proposed Grouped Accessors and Auto-Accessors proposal. Like prop, users would be able to modify a field declaration with { get; set; } to create getters and setters on it. prop would effectively become syntactic sugar for this style of declaration.

The reason we decided not to use the auto-accessor syntax was so that we aren't blocked on that proposal, since it is currently in stage 1, and because the syntactic sugar of the keyword was seen as a good thing among users who will have to use prop to convert their existing decorators.

But yes, if you compose decorators, and one of them adds prop, then the rest need to handle that, just like if a class decorator changes the .name or identity of a class, the rest of the decorators need to handle that.

So let's dig in a bit more there. If this is the case, then all three of these would be a valid way to get a prop, and effectively be equivalent, no?

@prop: a @b @c x = 123; @a @prop: b @c x = 123; @a @b @prop: c x = 123; // Can also call it on all of them @prop: a @prop: b @prop: c x = 123

It seems like this creates more confusion than it helps, again because the transform is not coming from the decorator itself, its coming from the base element being changed.

It's coming from the way the decorator is called, essentially, yes - just like the this value and the arguments of a function do.

hmm, I think there may be a bit of a miscommunication. Are you suggesting that the value would only change after the @prop: decorator? e.g.

@a // prop @prop: b //prop @c // field x = 123;

That feels like it would make a bit more sense. I still think it would be a major refactoring hazard though

Another possibility would be to allow prop and field decorators to interleave, and call field decorators as field decorators instead:

@a // kind: 'field' @prop:b // kind: 'prop' @c // kind: 'field' x = 123;

I think this might work, since the field decorators would just be returning initializer functions, which could be tacked onto the private slot for the prop

ljharb · 2021-01-22T04:27:12Z

README.md

+```ts
+type ClassInitMethodDecorator = (value: Function, context: {
+  kind: "init-method";
+  name?: string;


Suggested change

name?: string;

name?: PropertyKey;

ljharb · 2021-01-22T04:27:44Z

README.md

+
+#### Class Initialized Methods
+
+Class initialized methods are a new construct, defined by adding the `init` keyword in front of a class method:


What are the use cases for this in the absence of a decorator?

This is what I'm more suspicious of; I'd prefer we stick with @init:.

Similar to the prop keyword, the main point is to change the semantics of methods so that decorators can layer naturally on top, replacing a init-method with another init-method. This is important for use cases such as @bound, where some initialization logic needs to run with the method.

Doing it this way keeps it consistent with the strategy for prop, so we don't have two different ways of introducing alternative semantics for decoratable values. @rbuckton also thought that this made more sense than @init:, because it does change the semantics of the element.

I agree though that the potential use cases for init on its own are even more limited than the use cases for prop on its own.

Changing the semantics in a decorator is fine tho, the issue was about not being able to statically know how the semantics were being changed.

ljharb · 2021-01-22T04:28:44Z

README.md

 }
+
+C.prototoype[Symbol.metadata]["#x"];


What happens if i also have @myMeta ['#x'] = whatever; on the same class?

Ah, this is a great point, I hadn't considered that. I think we'll need to change this somehow to account for that.

pabloalmunia · 2021-01-22T08:55:44Z

README.md

+      const ret = value.call(this, ...args);
+      console.log(`ending ${name}`);
+      return ret;
+    };


Caution with arrow function and this.

function logged(value, { kind, name }) { if (kind === "method") { return function (...args) { // Must be a regular function console.log(`starting ${name} with arguments ${args.join(", ")}`); const ret = value.call(this, ...args); // this is used here console.log(`ending ${name}`); return ret; }; } }

littledan · 2021-01-22T11:50:18Z

README.md

-// ending m
+  ```js
+  class Example {
+    @bound init myMethod() {


What was the motivation for using this syntax rather than @init: bound? I got a lot of feedback in the direction of preferring the @init: syntax.

The main reason is for consistency with the prop keyword and strategy for changing semantics of a decorator target. If we were to introduce @init: syntax, it would mean adding two separate ways to do this, and the rules for applying @init: seem less well defined.

One possible benefit of @init: I suppose is that it could also apply to other decorators. For instance, class decorators could use it, which would solve some of the problems brought up in #329:

@init: defineElement('my-element') class MyElement extends Element {}

littledan

Some initial comments, but this PR is a big improvement--thanks for putting in this work.

littledan · 2021-01-22T11:52:07Z

README.md

+
+In general, decorators receive two parameters:
+
+1. The value being decorated, or `undefined` in the case of class fields which are a special case (see below for details).


You could make this sound smoother by saying "undefined in the case of class fields, as these don't have a corresponding shared value" (a similar amount of text but less scary-sounding)

littledan · 2021-01-22T11:52:34Z

README.md

+```ts
+type Decorator = (value: Input, context: {
+  kind: string;
+  name?: string;


Agreed, or alternatively, string | symbol to make it more readable for people who might not know the jargon.

littledan · 2021-01-22T11:54:19Z

README.md

+  - `"class"`
+  - `"method"`
+  - `"init-method"`
+  - `"getter"`
+  - `"setter"`
+  - `"field"`
+  - `"prop"`


private is an orthogonal property that can apply to almost all of these. I think it'd be a bit awkward if you were forced to parse the string, if we had, e.g., "public-method" vs "private-method", to figure out if it's a method at all.

littledan · 2021-01-22T11:57:18Z

README.md

+type ClassGetterDecorator = (value: Function, context: {
+  kind: "getter";
+  name?: string;
+  access?: { get?(): unknown };


This is basically to do the private brand check, right? It'd probably be good to clarify that somewhere.

littledan · 2021-01-22T12:47:45Z

README.md

+- `access`: An object containing methods to access the value. This is only available for _private_ class elements, since public class elements can be accessed externally by knowing the name of the element. These methods also get the _final_ value of the private element on the instance, not the current value passed to the decorator. This is important for most use cases involving access, such as type validators or serializers. See the section on Access below for more details.
+- `isStatic`: Whether or not the value is a `static` class element. Only applies to class elements.
+- `isPrivate`: Whether or not the value is a private class element. Only applies to class elements.
+- `defineMetadata`: Allows the user to define some metadata to be associated with this property. This metadata can then be accessed on the class via `Symbol.metadata`. See the section on Metadata below for more details.


Is this a new closure each time, or does it read an internal slot of the context object? (Either answer seems OK to me, but we have to decide.)

littledan · 2021-01-22T12:53:43Z

README.md

 ```

-### Limited access to private fields and methods
+Properties, unlike fields, define a getter and setter on the class prototype. This getter and setter default to getting and setting a value on a private slot. The above roughly desugars to:


The answer to your question about how an undecorated prop differs from a public field in semantics is below in this document. As for use cases: I could imagine using prop fields to faithfully emulate other cases in JS and the web platform where such patterns are used by built-in features. But really, what brings them over the bar for being "worth it" to add to the language is decorators, IMO.

I think @decorator prop field; makes a lot more sense than @prop: decorator field; since prop changes the shape of the class in a way that's unrelated to the application of any one particular decorator. (I guess init in this proposal has that same property, though.)

littledan · 2021-01-22T13:08:03Z

README.md

+
+#### Class Initialized Methods
+
+Class initialized methods are a new construct, defined by adding the `init` keyword in front of a class method:


This is what I'm more suspicious of; I'd prefer we stick with @init:.

littledan · 2021-01-22T13:08:34Z

README.md

 }
 ```

-This example could be roughly desugared as follows:
+Initialized methods are methods that are defined on the prototype, but then set as an instance property. The above roughly desugars to:


Is this a commonly needed thing, or just solving for @bound?

There is also the event handler decorator which @justinfagnani has raised before, and @leobalter raised the @wire decorator provided by Salesforce's component library (scroll down to "Decorate a Function with @wire").

littledan · 2021-01-22T13:25:02Z

README.md

+  qux: { "my-meta": true, [MY_META]: true },
+};
+
+C.prototype[Symbol.metadata] = {


How are fields and methods with the same name differentiated?

My thinking was that they would not be, given this is not an extremely common pattern, and init-methods make it even less necessary. Decorators on elements of the same name would be coalesced, and if your decorator needed to do something specific for the field or the method, it could include the value it decorated in the metadata itself (e.g. using kind).

I think @ljharb's point about the collision between private fields and dynamic public fields is more of a problem though, since those are actually different names, one is just the "spelling" of the private field but isn't at all related to the public field. I think that means that we'll need to introduce namespacing for private values no matter what, so we could consider namespacing different types of elements as well. I was mainly worried about a combinatorial explosion there (private fields, public fields, private methods, public methods, etc...) but maybe that is not avoidable.

littledan · 2021-01-22T13:26:19Z

README.md


-If it's not acceptable to require a superclass/mixin for cases requiring initialization action, then the `@init:` decorator syntax in a method declaration allows decorators to add initialization actions, run when the constructor executes.
+#### Privatizing metadata


Suggested change

#### Privatizing metadata

#### Hiding metadata

Updates the README with a more detailed breakdown of the API, along with a number of tweaks to the APIs including: - Adds the `prop` keyword and class element type - Adds the `init` keyword and class element type - Updates field semantics to not transform to getter/setter pairs by default - Adds the `access` property to the context object for private elements - Refactors the metadata API to be more intuitive and well defined

leobalter · 2021-01-25T18:51:23Z

Considering the last README is outdated and there are just so much we need to discuss for the current README, I'd suggest we just have this PR landed and break outstanding comments into separate issues.

I've seen so much already for feedback I don't feel encouraged to add anything else to be fixed or changed in this single PR. Formatting wise, it seems fair enough. Generally I like the current direction this is going.

pzuraq · 2021-01-29T23:07:10Z

Discussed this with @littledan and @leobalter and we agreed it makes sense to merge this and continue to iterate on the open questions. I'll open up new issues to discuss the points brought up by @ljharb and others, and we'll add them to the agenda for the next decorators meeting.

chriskrycho reviewed Jan 22, 2021

View reviewed changes

README.md Show resolved Hide resolved

ljharb requested changes Jan 22, 2021

View reviewed changes

pabloalmunia reviewed Jan 22, 2021

View reviewed changes

littledan reviewed Jan 22, 2021

View reviewed changes

pzuraq force-pushed the update-readme-to-include-prop-init branch from 2276f88 to fba22a0 Compare January 25, 2021 18:08

pzuraq force-pushed the update-readme-to-include-prop-init branch from fba22a0 to 6f4409d Compare January 25, 2021 18:21

pzuraq merged commit eb96129 into tc39:master Jan 29, 2021

ljharb mentioned this pull request Jan 30, 2021

How is metadata stored for private fields and public fields with conflicting names? #357

Closed

	- Class _properties_, defined by applying the `prop` keyword to a class field. These have a getter and setter, unlike fields, which default to getting and setting the value on a private storage slot (equivalent to a private class field):
	- Class _properties_, defined by applying the `prop` keyword to a class field. Unlike fields, these have a getter and setter, which default to getting and setting the value on a private storage slot (equivalent to a private class field):


		#### Class Initialized Methods

		Class initialized methods are a new construct, defined by adding the `init` keyword in front of a class method:


		In general, decorators receive two parameters:

		1. The value being decorated, or `undefined` in the case of class fields which are a special case (see below for details).


		If it's not acceptable to require a superclass/mixin for cases requiring initialization action, then the `@init:` decorator syntax in a method declaration allows decorators to add initialization actions, run when the constructor executes.
		#### Privatizing metadata

		}

		C.prototoype[Symbol.metadata]["#x"];

Update README with more details #354

Update README with more details #354

Conversation

pzuraq commented Jan 22, 2021

Choose a reason for hiding this comment

chriskrycho Jan 22, 2021 • 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

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

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

Choose a reason for hiding this comment

pabloalmunia Jan 22, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

littledan 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

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

leobalter commented Jan 25, 2021

pzuraq commented Jan 29, 2021

chriskrycho Jan 22, 2021 •

edited

Loading

pabloalmunia Jan 22, 2021 •

edited

Loading