js-classes-1.1 icon indicating copy to clipboard operation
js-classes-1.1 copied to clipboard

Can we define the rules for this proposal?

Open rdking opened this issue 5 years ago • 42 comments

Ok. In #45, it looks like we more or less have an idea of what a class with private data should look like. What do we do about static? Should static hidden functions and data exist? What does that mean? Or is that a "save it for later" topic?

So far, it looks like:

  • define hidden data using let or const as you would with function closure
  • define hidden functions using function as you would with a closure
  • public functions can access hidden data of the current context by name directly
  • public functions can access hidden data of a sibling or descendant context via :: (closure access operator)
  • public functions can access hidden functions of the current context by name directly if that function does not need a context
  • public functions can access hidden functions of the current, a sibling, or a descendant context via :: to apply that context to the call.
  • public properties are accessors functions that also follow the above rules

Does this look right so far?

rdking avatar Oct 19 '18 05:10 rdking

Here's my 2 cents on this in code form:

class Point2D {
  const POLAR = Symbol();
  const XY = Symbol();
  let x = 0;
  let y = 0;
  function polarToXY(r, theta) {
    return { x: r * Math.cos(theta), y: r * Math.sin(theta) };
  }

  get POLAR() { return POLAR; }
  get XY() { return XY; }
  get x() { return x; }
  get y() { return y; }
  translate(type, ...args) {
    switch (type) {
      case POLAR: {
        let p = polarToXY(args[0], args[1]);
        x += p.x;
        y += p.y;
        break;
      }
      case XY: {
        x += args[0];
        y += args[1];
      }
      default:
        throw new TypeError("Invalid coordinate type");
    }
  }
  constructor(x, y) {
    this::x = x;
    this::y = y;
  }
}

class Vector2D {
  let point = new Point();
  let magnitude = 1;
  let direction = 0;

  get position() { return Object.assign({}, point); }
  get magnitude() { return magnitude; }
  get direction() { return direction; }

  scale(factor) {
    magnitude *= parseFloat(factor);
  }
  turnTo(angle) {
    direction = parseFloat(angle);
  }
  turnBy(angle) {
    direction += parseFloat(angle);
  }
  translate(...args) {
    point.translate(...args);
  }
  constructor(pos, mag, dir) {
    point = pos;
    magnitude = mag;
    direction = dir;
  }
}

Beautiful, isn't it?

rdking avatar Oct 19 '18 05:10 rdking

@rdking

What do we do about static? Should static hidden functions and data exist? What does that mean? Or is that a "save it for later" topic?

The "closure" answer would be that we can use the surrounding scope for "private static" stuff:

let staticData = 1;
class C {
  let instanceData = 2;
}

similar to how we would do such things in ES5:

var staticData = 1;
function C() {
  var instanceData = 2;
  return {};
}

Would that be sufficient?

I'm a little concerned about having nested function declarations in the class body, for a couple of reasons.

First, since we would have two ways to define "functiony" things inside of the class body it might be a little confusing as to which is appropriate (for newer users):

class C {
  // Which one is the "right" one? What's the difference?
  // Do I need to go to StackOverflow?
  function f() {}
  f() {}
}

Second, nested function would close over the instance variables, but not over this (because they aren't arrow functions):

class C {
  let x = 1;
  function f() {
    assert(x === 1); 
    assert(this === undefined);
  }
  constructor() {
    f();
  }
}

For these reasons it might be better overall to require the use of arrow functions (which don't have the this confusion issues):

class C {
  let f = () => {
    assert(x === 1);
    assert(typeof this === 'object'); // Yay
  };
  constructor() {
    f();
  }
}

Also, devs are kinda using this arrow function pattern with public class fields (via Babel) already.

Beautiful, isn't it?

I think so 😉

zenparsing avatar Oct 19 '18 06:10 zenparsing

@zenparsing

The "closure" answer would be that we can use the surrounding scope for "private static" stuff

Would that work? A problem with your example is that since the "closure" is contained between the {}, staticData wouldn't be within the closure owned by the class, let alone the constructor. We really want to encode something like this:

class C {
  {
    let staticData = 1;
    ::constructor() {}
  }
}

Of course, I wouldn't suggest that syntax, but a scope enclosing the static private data and the constructor is what's needed. I'm thinking about syntax like:

class C {
  static {
    let staticData = 1;
  }
  constructor() {}
}

rdking avatar Oct 19 '18 17:10 rdking

I imagine that it would follow the same scoping rules as usual, and any bindings defined outside of the class definition would be available as usual from the inside.

let staticData = 1;
export class C {
  static getStaticData() {
    return staticData;
  }
}

// --- Elsewhere --

import { C } from './C.js'
assert(C.getStaticData() === 1);

zenparsing avatar Oct 19 '18 17:10 zenparsing

I see what you're doing, but that's module level. If multiple classes were defined in the same module, they would all share access to variables that are supposed to be private to the separate class constructors. Not the intended effect. It's a common idiom for module private, but the goal is class private.

rdking avatar Oct 19 '18 20:10 rdking

@zenparsing Looking back over your arguments again:

First, since we would have two ways to define "functiony" things inside of the class body it might be a little confusing as to which is appropriate (for newer users):

I thought about this, too. That can be confusing, but...

Second, nested function would close over the instance variables, but not over this (because they aren't arrow functions)

... at first, I didn't want to use arrow functions because I was thinking to always have the developer specify the context using ::. Just now it hit me that if :: is a scope access operator, but the private functions are not bound to a context, then it becomes really awkward to call a method with a context assuming nothing else changes: obj::foo.bind(obj)(...args). So I say yes to arrow function privates instead of typical function declarations.

rdking avatar Oct 23 '18 14:10 rdking

From above, a rewrite:

class Point2D {
  const POLAR = Symbol();
  const XY = Symbol();
  let x = 0;
  let y = 0;
  //This private method is now...
  let polarToXY = (r, theta) => ({ x: r * Math.cos(theta), y: r * Math.sin(theta) });

  get POLAR() { return POLAR; }
  get XY() { return XY; }
  get x() { return x; }
  get y() { return y; }
  translate(type, ...args) {
    switch (type) {
      case POLAR: {
        let p = polarToXY(args[0], args[1]);
        x += p.x;
        y += p.y;
        break;
      }
      case XY: {
        x += args[0];
        y += args[1];
      }
      default:
        throw new TypeError("Invalid coordinate type");
    }
  }
  constructor(x, y) {
    this::x = x;
    this::y = y;
  }
}

This works since the context of the scope being accessed is always the instance object you'd apply :: to for accessing siblings.

rdking avatar Oct 23 '18 14:10 rdking

After re-thinking private functions, the static picture looks a little different to me now. How about this?

class Ex {
  /* I don't like public data properties, but...  */
  prop1 = 1; //public property
  static prop2 = 1; //static public property
  /* the private data types... */
  let prop3 = 2; //private member
  let static prop4 = 3; //static private member
  const prop5 = 5; //constant private member
  const static prop6 = 8; // constant static  private member
}

The only thing that's missing is a way to do constant public, but that makes little to no sense to me.

rdking avatar Oct 23 '18 15:10 rdking

A const private member makes little sense to me, since it’s only your own code that could be reassigning it - whereas a const public member is preventing the outside world from writing to it. const public would be much more important imo than const private.

ljharb avatar Oct 23 '18 15:10 ljharb

@ljharb Why have a public data member if the only thing you're going to do is read it? Declaring a constant internal variable is good for cases where certain values are key and absolutely must not change during the runtime of the code. It also signals to future developers that this is a "change at your own peril" kind of thing. Isn't const-public better handled by a getter with no setter so that it's an obvious read when you look at the code, and you get an error if you don't read the code and try to set?

Wait, the more I think about it, the more of a wash it becomes. I'm already biased against public data properties in the presence of private data. However, I have to apply my own rule of "minimum opinionation". Can you think of a decent way to have const be usable on both public and private without running into AST problems? The only thing I can come up with is let const propX, but that leads to the excessive (and probably extremely edge case) let const static propY.

rdking avatar Oct 23 '18 16:10 rdking

Nothing is better handled by a getter imo - a nonwritable public data property is already a thing in the language, and is quite useful for exposing constants to the world.

I think anything you can do on private you should be able to do in public, and anything you can do on the instance you should be able to do on static. The best option i can think of to ensure that symmetry and avoid confusion would be some kind of syntax to differentiate - a sigil, perhaps.

ljharb avatar Oct 23 '18 16:10 ljharb

Nothing is better handled by a getter imo

That's one of the reasons why I apply the rule of "minimum opinionation". If I keep things as un-opinionated as possible, I greatly reduce the risk that I'll have painted my future-self into a corner due to my present opinion on things.

anything you can do on the instance you should be able to do on static.

One place where I fear we might always disagree is about having class be able to manipulate things that are not its products. I wouldn't mind going a few rounds with you on that subject like last time. However, for now I will say that it's more inline with the existing language to say that "anything you can do on the prototype you should be able to do on static (the constructor)." That fits how class currently works.

The best option i can think of to ensure that symmetry and avoid confusion would be some kind of syntax to differentiate - a sigil, perhaps.

Definition by sigil is probably the most un-ES like thing about the class-fields proposal. So that's out. Definitions in ES (save for the one weird case of keyword-less definition) require keywords. let, const, & static already have meanings that should not drift too far away from their present meanings. Ignoring methods, the only properties presently in a class use get/set to define them. Since these words come from property descriptors, how about we take our cue from that? Since writable = true by default, what if the keyword is readonly?

class Ex {
  /* I don't like public data properties, but...  */
  prop1 = 1; //public property
  static prop2 = 1; //static public property
  readonly prop3 = 2; //non-writable public property
  readonly static prop4 = 3; //non-writable public static property
  /* the private data types... */
  let prop5 = 5; //private member
  let static prop6 = 8; //static private member
  const prop7 = 13; //constant private member
  const static prop8 = 21; // constant static  private member
}

What about this?

rdking avatar Oct 23 '18 18:10 rdking

Why would "readonly" only apply to public but "const" to private? I'd expect to be able to declare readonly private things.

ljharb avatar Oct 23 '18 18:10 ljharb

@ljharb Working on the previous post made me think of something. Is there a single term describing the collection of all things that can access or are in the private data space of a class instance? I was thinking they should be called "members". From the point of view of class-fields, members are everything within the lexical scope of the declaration.

From the point of view of this proposal, it would be the same, with a minor exception. With this proposal, members aren't necessarily properties of the instance or class(prototype & constructor). However, like class-fields, not all properties are members since new properties can be tacked onto the class or instance at any time. Unless there's something wrong with what I'm thinking, this should make conversations and explanations a little easier.

rdking avatar Oct 23 '18 18:10 rdking

@ljharb

Why would "readonly" only apply to public but "const" to private? I'd expect to be able to declare readonly private things.

Part of the difference is in how this proposal would operate. The class keyword under this proposal would have 3 products instead of 2. The new product is a closure initializer that is run on instantiation before the constructor. In fact, you could think of it as a function that is called by new that calls the constructor after staging members. This means that the member definitions should be as similar as possible to normal variable definitions in a normal function closure.

Another reason is to avoid AST problems. Both 'let' and 'const' are seen like 'var' to the language, so they're not meant to be stacked. I don't want to change that. Further, it's already well established that in any closure, const is how to specify a read-only variable. However, a read-only property is a different kind of thing. The difference in keyword will make the AST easier to deal with, while at the time visibly signaling to developers that this is a property and not just another member of the closure.

rdking avatar Oct 23 '18 18:10 rdking

Would I then be able to do readonly foo() {}, static readonly bar() {}?

ljharb avatar Oct 23 '18 18:10 ljharb

@ljharb Let me see if I understand the question: you're asking if it would be ok to mark functions on the prototype as not writable? Sure, why not? It's actually possible, so there's no need to limit it.

rdking avatar Oct 23 '18 20:10 rdking

In that case, I'd say before this proposal used readonly, we'd need to add readonly as a generic class member modifier - at which point, using it here (or in the current stage 3 proposal) would fit intuitively.

ljharb avatar Oct 23 '18 20:10 ljharb

@rdking

After re-thinking private functions, the static picture looks a little different to me now. How about this?

class Ex {
  /* I don't like public data properties, but...  */
  prop1 = 1; //public property
  static prop2 = 1; //static public property
  /* the private data types... */
  let prop3 = 2; //private member
  let static prop4 = 3; //static private member
  const prop5 = 5; //constant private member
  const static prop6 = 8; // constant static  private member
}

We'd better resist the temptation of adding too many features, or we'll face similar issues like current field proposal.

For example, private static fields in current proposal have the issue that subclass throw TypeError when calling static methods inherited from base class which access private static fields.

hax avatar Oct 24 '18 08:10 hax

@ljharb

Nothing is better handled by a getter imo - a nonwritable public data property is already a thing in the language, and is quite useful for exposing constants to the world.

If you only consider an object, non-writable data property is enough. But if you consider class hierarchy, the conflict of prototype-based accessor with ownproperty-based property is inevitable, I'm afraid. This is the consequence of ES6 class design, which is the consequence of the ES5 accessor design + prototype nature from the first day of JS.

hax avatar Oct 24 '18 09:10 hax

@ljharb

Would I then be able to do readonly foo() {}, static readonly bar() {}?

Unfortunately, readonly foo() {} will be confusing to average programmers. I believe most programmers think readonly should only apply to data property.

Though technically speaking, methods in JS class is just a property on prototype of the class, and the property is writable and configurable due to the requirements of keeping language dynamic and hackable (allow overwriting of prototype methods anyway), I'm afraid most programmers think methods as immutable/const thing.

hax avatar Oct 24 '18 09:10 hax

Should there also be a final that sets configurable to false?

rdking avatar Oct 24 '18 15:10 rdking

@hax

For example, private static fields in current proposal have the issue that subclass throw TypeError when calling static methods inherited from base class which access private static fields.

That's an implementation issue that's easy to work around. The mistake there is that private fields are not inheritable. However, when the static method is called against the subclass, it only has the subclass to reference for finding the private static fields. Unfortunately, those objects only live on the base class. The solution is to remember to always call static methods in the context of the owning class. Kind of a pain for deep hierarchies though. Might be better to just bind static functions to their owning class.

Unfortunately, readonly foo() {} will be confusing to average programmers. I believe most programmers think readonly should only apply to data property.

Precisely right, and that's what public properties would be under this proposal. I can't see adding "fields" to this proposal in any way. Not to mention that this proposal already makes it clear that "field" is a non-starter. However, splitting the difference isn't bad. Put the property on the prototype with the default value, and an assignment in the constructor if the default value is an object. That way the pseudo-static foot-gun is removed while still allowing derived class instances to provide functionality to any expecting base class. This will fully provide for the ability to implement base classes with abstract functionality.

rdking avatar Oct 24 '18 15:10 rdking

That can’t really be statically enforced - what about a value of Symbol() or returnsPrimitive() versus returnsObject()?

ljharb avatar Oct 24 '18 15:10 ljharb

All function calls would be evaluated at the time of parsing of the class definition. So at that time, the decision would be made. If something is meant to have a more dynamic effect (potentially rendering a different result for each instance) then it needs to be directly placed in the constructor by the developer. Where ES is concerned = is always immediate.

rdking avatar Oct 24 '18 16:10 rdking

Honestly, I'd rather bind a rule to the declaration like "only primitives and frozen objects can be assigned to data properties in a class declaration". That would remove the foot-gun since most of the time when it happens, it's because someone assigned a mutable object to the prototype with the intention of populating it later. For the cases of 2nd degree property mutation (prop of a prop), there's little that could be done. However, the simple fact that objects must be frozen will tend to help developers realize they shouldn't be doing what they're attempting unless they really want the pseudo-static effect (which I think there are still a small set of use-cases for).

rdking avatar Oct 24 '18 17:10 rdking

A frozen Map or Set is still mutable, as are many other builtins. I don’t think that’s a viable approach.

ljharb avatar Oct 24 '18 17:10 ljharb

In this case, the goal isn't so much about making it impossible to trigger the foot-gun as it is about forcing the expectation that a certain practice is usually a bad idea. If you wanted to re-shape the goal to eliminate the possibility of the foot-gun, not only do you wind up breaking a rarely useful (but still useful) feature, you also end up causing other problems unexpectedly. I've been repeatedly raising these issues in class-fields.

That's why I decided to take the first approach. It reasonably avoids the foot-gun without removing it and/or breaking other features, all the while keeping public property decorators completely functional. I'm still trying to figure out whether or not its worth while to make decorators work with private members in this proposal. Can you give me a realistic example of why a developer would want to use a decorator on a private member?

rdking avatar Oct 24 '18 17:10 rdking

There are tons of use cases for decorating a private field; largely the same ones for public fields.

The bad idea, as decided by overwhelming community usage and idiom, is putting data properties of any kind on the prototype - you continue to argue for that very thing.

ljharb avatar Oct 24 '18 17:10 ljharb

I argue for it because it's in the same class as a variable declaration without keyword ending up on global, generally a bad idea but it has it's uses. Plus, in the same way, this feature pre-existed class, so it's not something that should even be considered for removal or hindrance. Not putting properties on the prototype is throwing out the baby with the bath water. The problem isn't the existence of properties on the prototype, but rather the simple fact that the objects on the prototype can have their own properties mutated without triggering duplication of the object into an own property of the instance having that prototype. That doesn't happen if the value of the property is a primitive or a frozen object with no nested objects or internal slots.

My argument is that we should be far more surgical in what we block. As for what the community is doing, they can keep their idiom as long as I don't have a feature I use snatched from under me do to using such broad strokes to paint a small problem.

rdking avatar Oct 24 '18 18:10 rdking