JS Class fields potentially harmful

[WebReflection]

OK, I've read it all, thanks for sharing, but it's not super related ...

• the sub-class method accesses a property that doesn't even exist on the super so that's an expected shenanigan
• the sub-class constructor explicitly set helper after, while here everything is implicit and non fine-tunable from the author of the class which misleads results (WYSI-NOT-WYG)
• there's no abstract in JS, meaning in JS nobody can really signal a class is meant to be subclassed or not ... but even if abstract was a thing, the issue presented here has nothing strictly to do with methods (I am considering accessors methods too, even if as workaround these work)
• the key to discuss, which I did in my medium's post, is the final keyword ... allow me to expand

class Point {
  x = 0; // this is final !!!
  y = 0; // this is not
  xRelated; // dummy example
  constructor() {
    // x is final because it's used in the constructor
    // y is not final because it's *not* used in here
    // dummy example that uses the x for whatever reason
    this.xRelated = `point x: ${this.x}`;
  }
  getCoords() {
    return [this.x, this.y];
  }
}

If we had a final syntax/keyword, we can signal which properties should never be overridden and which are safe to assume will always work as expected or can be accessed without issues ... let me get to the point:

class ThePoint extends Point {
  x = 1; // this *should* throw
  y = 1; // this is fine
  constructor() {
    super();
    console.log(this.xRelated); // "point x: 0"
    console.log(this.getCoords()); // [1, 1]
  }
}

In few words, the x used in the constructor has no way to signal that it should never be overridden at the sub class definition, while the Java example you linked has this ability via private final FooBar fooBar;

Now, assuming we have no way to fix this, can we at least consider bringing final at least for classes fields?

@bakkot

the superclass constructor could be function(){ return {}; }, so how could you possibly install fields on the instance before invoking the superclass?

So now we talk, or better, now we have a technical reason to justify the current approach, hence something to discuss ... and yet your argument is very weak, because if function () { return {} } every super field is fully ignored, while sub-class fields will be attached:

class Root {
  kind = 'wood';
  constructor() {
    // this is the B method from Sub when `new Sub`
    // and `this` is already an instanceof Sub
    this.init();
    // we throw away the instance and expect this
    // array to be enriched with fields once landed because ... 
    return [];
  }
  init() {
    console.log('I am root 🪵');
  }
}

class Sub extends Root {
  kind = 'mermade';
  constructor() {
    // here the instanceof Sub *already exists* but
    // it cannot be accessed unless super() happens
    super();
    // super overrided the return type
    console.log(this.init); // undefined
  }
  init() {
    console.log('I am sub 🏊');
  }
}

(new Sub).kind;
// logs 'I am sub 🏊' then `undefined`
// but it also returns marmade

// we completely lost the inheritance
new Sub instanceof Sub;   // false
new Sub instanceof Root;  // false

Basically this example shows that using a different instance as returned value in the super constructor accidentally pollutes such value with classes fields that don't belong to it (Liskov substitution principle anyone?), like the init method doesn't belong to it and anything else in the class. Indeed the class inheritance is broken and the fields attached to something else that is not the class instance these are supposed to be attached.

A great example of how mind twisting is the current approach to me, rather than a valid point to justify the current state ... let's put it down in booleans:

class A {
  static value = false;
  get value() { return false; }
  shenanigan = false;
  constructor() {
    console.log(
      this.constructor.value, // true
      this.value,             // true
      this.shenanigan,        // false
    );
    return {};
  }
}

class B extends A {
  static value = true;
  shenanigan = true;
  get value() { return true; }
  constructor() {
    super();
    console.log(
      !!this.constructor.value, // false
      !!this.value,             // false
      this.shenanigan,          // true
    );
  }
}

new B;

so ... back to the question:

@bakkot

how could you possibly install fields on the instance before invoking the superclass?

You don't, because the instance is lost in this case so that nothing undesired or surprising happens, as it is now instead.

edit by "you don't" I mean that it's not an issue, you just install fields to the currently created instance right before super call which is always safe and what developers expect ... what happens with the returned value, if changed, is a developer choice, not a problem to solve ... and no developer would expect a differently returned value to be enriched only with classes fields it doesn't belong to instead of being, dunno, upgraded as instanceof the Class.

[WebReflection]

I went ahead and tested Java, as it's been brought up as reference:

class A {
  String value = "a";
  String common = "ok";
  public A () {
    System.out.println(this.value);
    System.out.println(this.common);
  }
  public static void main(String[] args) {
    B b = new B();
  }
}

class B extends A {
  String value = "b";
}

// a
// ok

I feel like we're comparing apples to oranges though, because it's impossible to pollute/leak class fields to types that don't belong to the class, as AFAIK constructors are not allowed to return anything at all.

On the other hand, JS ain't Java, and we have an opportunity to make sense of the class fields so that nobody needs to explain anything around issues these cause to either returned overrides, that have nothing to do with the class and its inheritance chain, or during construction, where the parent constructor could be simply borrowed.

In doing so, explicit intents of setting other fields after the super() call happens are allowed and visible, but the sane expected default with class fields is that nothing leaks or pollutes possible returned values that don't belong to the class and developers, in this way, have full control of the code they wrote.

Currently this control is missing and extremely hard to reason about + it's a very leaky and counter-intuitive approach that will haunt the feature forever if it stays the way it is.

As Summary

This is how I expect the code should look like, once transformed/transpiled:

// developer intent
class A {
  value = 'a';
  constructor() {
    console.log(this.value);
  }
}

class B extends A {
  value = 'b'; // explicit override at class definition
  constructor() {
    super();
    // explicit override after super call
    // leaks can happen but *can* be handled too
    this.extra = 'c';
    console.log(this.extra);
  }
}

// above code should become something like:
function addFields() {
  for (const [key, value] of arguments) {
    if (!Object.hasOwn(this, key)) {
      Object.defineProperty(this, key, {
        configurable: true,
        enumerable: true,
        writable: true,
        value
      });
    }
  }
};

function A() {
  addFields.apply(this, [['value', 'a']]);
  const self = this;
  console.log(self.value);
  return self;
}

function B() {
  addFields.apply(this, [['value', 'b']]);
  const self = A.apply(this, arguments);
  self.extra = 'c';
  console.log(self.extra);
  return self;
}
Object.setPrototypeOf(B, A);
Object.setPrototypeOf(B.prototype, A.prototype);

new B;
// b
// c

[WebReflection]

last about leaks:

class A {
  constructor() {
    return new String("");
  }
}

class B extends A {
  #secret = Math.random();
}

new B;

check the console and expand the String reference:

secrets are not accessible but somehow inspectable and I doubt any developer would expect this to happen.

considering that "private fields can't be deleted" neither, this hack could cause issues, imho.

if fields were attached before the super() executes, this leak wouldn't be possible.

[WebReflection]

Alternative Proposal

Instead of playing before/after super(...args) calls, move classes fields into the protoype and make all of them lazy accessors:

class A {
  value = 'a';
  constructor() {
    console.log(this.a);
  }
}

class B extends A {
  value = 'b';
}

// translates into

const lazyField = ({prototype}, key, value) => {
  Object.defineProperty(prototype, key, {
    // set as own property when accessed
    get() {
      if (!prototype.isPrototypeOf(this))
        throw 'go away';
      return (this[key] = value());
    },
    // set as own property when set
    set(value) {
      if (!prototype.isPrototypeOf(this))
        throw 'go away';
      Object.defineProperty(this, key, {
        writable: true,
        enumerable: true,
        configurable: true,
        value
      });
    }
  });
};

function A() {
  const self = this;
  console.log(self.value);
  return self;
}

lazyField(A, 'value', () => 'a');

function B() {
  const self = A.apply(this, arguments);
  return self;
}

lazyField(B, 'value', () => 'b');

Object.setPrototypeOf(B, A);
Object.setPrototypeOf(B.prototype, A.prototype);

new B;
// b

This would preserve the expectations developers coming from Python, PHP and others to understand that fields are being set at the prototype level hence their value during constructor time comes directly from the prototype and it sets it once accessed (common case when value is known) or when set (fields defined without a value use case).

The only gotcha here is the observability at the own property level when a field hasn't been set or accessed, but it feels more like an in-depth implementation detail, than a real-world footgun to deal with.

This would also avoid any debate like "JavaScript did it better than Java" or "JavaScript is changing Java's OOP" because the proposal is about "JavaScript followed other scripting languages class definition expectations making these even more powerful as any data, even non primitive, can be defined in the class without sharing such reference across all instances (but you can define shared references manually via Class.prototype.shared = {any: "data"} as there's no native syntax yet to do so anyway)".

This also solves any leak previously discussed.

I don't know how much performance could be impacted, but a lazy Object.defineProperty VS an always (leaky) Object.defineProperty doesn't look like a concern to me.

P.S. there's no confusion about actual shared references at the prototype level, because there is no syntax to define these beside accessors and methods ... fields shared at the prototype level don't exist, so this idea doesn't conflict with anything already knew before ... it's a better way to provide fields that comes from instances defined via new Anything always allowing intended overrides.

Heck this idea might even pave the path for a future shared field = 'value'; or any other keyword at class definition time ...

[aclaymore]

Another reason for the current semantics is when the engine generates code for class A it can generate code that makes more assumptions about the state of this because subclass constructors do not have access to the instance before the parent has completed their constructon.

This is somewhat similar to the limitations on decorators, the design was changed to reduce where shape changes or the instance can happen so engines can make more assumptions and generate more efficient code.

[WebReflection]

the shape changes regardless, just later, instead of right on instantiation ... actually, if shape matters, having sub classes shaped out of the box should be a performance win ... but look, I've played around C++ too (here for history sake):

#include <iostream>
#include <string>
using namespace std;

class A {
  public:
    string value = "a";
    string common = "ok";
    A() {
      cout << value + "\n";
      cout << common + "\n";
    }
};

class B: public A {
  public:
    string value = "b";
};

int main() {
  B b;
  return 0;
}

and yes, it looks like old typed OOP languages all follow the same principle but none of them have the function () { return {} /* or anything else */ } form the super() issue so that properties attached at distance to foreign objects is a non-existent concern plus they are not prototypal inheritance based, which is the nature of JS, not being Java or C++, being JS.

Python, PHP, imbajs, and likely anything more modern than Java or C++ and based on prototypal (like) inheritance understood that class fields should be retrieved through the prototype but here JS is making a step into big old strict/compiled OOP languages world with all its "whatever is returned from super" scripting abilities.

The footgun, in other scripting languages, is that a field that points at non primitve values can affect every instance (i.e. field = [] as array ... any instance pushing to that field will affect every other instance!).

That is something a new standard proposed for the most deployed PL out there could learn from and address, effectvely removing even that issue from happening in the wild! (make JS a better scripting choice is my motto here ... in case it wasn't clear ...)

If transpilers are the argument to use here, transpilers are also those leaking even private fields with ease:

// evil/unintentional code
const {set: _set} = WeakMap.prototype;
WeakMap.prototype.set = function set(key, value) {
  console.log('leaked', key, value);
  return _set.call(this, key, value);
};

// user code
class A {
  #secret = Math.random();
}

console.log(new A);

but the bigger issue here is that we're mimicking with JS PLs that have nothing to do with JS, ignoring what class definition fields look like in other scripting, OOP based PLs, such as Python and PHP.

Again, there is an opportunity to not disappoint expectations for both JS and other scripting PLs users plus the ability to make it even better, and if prototypal inheritance has worked to date for engines, having lazy classes fields only when a sub class override parent class fields doesn't look like the end of the world, actually it would perform like any accessor override already performs out there.

[WebReflection]

last stroke here ... decorators can't change this behavior neither, apparently, so that userland cannot fix this shenanigan even on super-explicit intent please fix classes fields already

[aclaymore]

Stage 3 decorators can provide something like final, and I think GitHub - tc39/proposal-decorator-metadata would make that a bit easier too.

A 'final' decorator

const fieldsToSet = new WeakMap();

function useFinal(klass, context) {
    console.assert(context.kind === "class");
    return class extends klass {
        static {
            Object.defineProperty(this, "name", {
                value: klass.name
            });
        }

        constructor(...args) {
            super(...args);
            const names = fieldsToSet.get(this) ?? [];
            fieldsToSet.delete(this);
            for (const name of names) {
                Object.defineProperty(this, name, {
                    configurable: false
                });
            }
        }
    }
}

function final(_, context) {
    console.assert(context.kind === "field");
    if (!context.static && !context.private) {
        const fieldName = context.name;
        return function (v) {
            let names = fieldsToSet.get(this);
            if (!names) {
                names = [];
                fieldsToSet.set(this, names);
            }
            names.push(fieldName);
            return v;
        }
    }
}

@useFinal
class X {
  @final
  prop = 2;
}

class Y extends X {
  prop = 3;
}

new X().prop; // 2
new Y(); // Error! Attempting to change configurable attribute of unconfigurable property. 

[WebReflection]

That would be something but my point around decorators is that apparently I cannot use decorators to define lazy accessors and be done with the issue as a whole.

class A {
  @lazy value = 'a';
}

class B extends A {
  @lazy value = 'b';
}

I couldn't find a way from the returned decorator for value to reach the class and/or remove classes fields from its initialization so there's no control for users, classes fields are non manageable with current decorators / primitives.

[ljharb]

You can do that with the init modifier.

[WebReflection]

Can I? Any example would be great, and if we can decide what lands on prototype and what is native field then we do have control indeed, so this hint to change native behavior remains but it becomes less of an issue, as there are workarounds.

[ljharb]

Ah, my mistake - a decorator on a class method can call addInitializer to install a private field, but to install a prototype thing you'd need to use a class decorator.

[aclaymore]

AFAIK there isn't a way to stop field syntax from resulting in it trying to define a property on this. Decorators don't change that.

Nor is there a way to access this before the parent constructor has returned, (unless that constructor calls out to something providing it with access to this).

If a fields initial value needs to be changed by a subclass, then the base class needs to explicitly support this as part of its API and take it as an argument to its constructor (the open-closed principle).

[WebReflection]

@aclaymore

AFAIK there isn't a way to stop field syntax from resulting in it trying to define a property on this.

that's indeed my understanding ... but ...

@aclaymore

the base class needs to explicitly support this as part of its API

it's not possible to do so via decorators ... I want to drop a field from a class as I need it to be a lazy field:

class Root {
  @lazy field = 'root';
  constructor() {
    // I want this to be 'leaf' when extended
    console.log(this.field);
  }
}

class Leaf extends Root {
  @lazy field = 'leaf';
}

even if I want to define at the root level a field is meant to be retrieved lazily from the prototype, is there any decorator logic able to do so? My understanding is that decorators don't play well with class fields, and class fields are shenanigans prone ... which summarizes pretty well this whole thread.

[WebReflection]

@aclaymore

the base class needs to explicitly support this as part of its API and take it as an argument to its constructor (the open-closed principle).

this doesn't work for the following case, presented already in the blog post:

class Counter extends HTMLElement {
  count = 0;
  onclick = () => { this.count += 1 };
  constructor() {
    this.addEventListener('click', this.onclick);
    // we're doomed: we can't remove this listener
    // developers expect this to be the inherited one
    // like it is for other scripting languages
  }
}

class DoubleCounter extends Counter {
  onclick = () => { this.count += 2 };
}

The field feature of encapsulating the context can't be substituted by parameters so the open-closed principle here doesn't apply.

Runtime methods that access private fields also are a very bad idea, not even possible, and an architecture smell too.

The workaround is worse DX (specially if the field is private, which cannot be with the current state of fields affair, not even via accessors, as accessors are retrieved by the sub class but the private field would be retrieved by the super).

edit in short there is no workaround and this would throw an error (bad example on purpose but the failure of current fields is even more evident here)

class Counter extends HTMLElement {
  #count = 0;
  #onclick = () => { this.#count += 1 };
  get onclick() { return this.#onclick }; // throws 
  constructor() {
    this.addEventListener('click', this.onclick);
  }
}

class DoubleCounter extends Counter {
  #onclick = () => { this.#count += 2 };
}

P.S. none of this would be a problem if fields were to be instantiated before the super or just retrieved by the class definition like everything else defined in the class.

[aclaymore]

But that class does not explicitly support onclick being changed, it might as well be a private field because it's read only once immediately on construction. So nothing can change it.

If Counter wants subclasses to customise the onclick behavior it could do this:

class Counter extends HTMLElement {
  count = 0;
  #incrementAmount; 
  constructor(options = {}) {
    this.#incrementAmount = options.amount ?? 1; 
    this.addEventListener('click', () => {
      this.count += this.#incrementAmount;
    });
  }
}

class DoubleCounter extends Counter {
  constructor() {
    super({ amount: 2 });
  }
}

Or use methods on the prototype, rather than fields:

`bind` decorator

function bind(_, context: ClassMethodDecoratorContext) {
    console.assert(context.kind === "method");
    console.assert(!context.static);
    console.assert(!context.private);
    const methodName = context.name;
    context.addInitializer(function() {
      this[methodName] = this[methodName].bind(this);
    });
}

class Counter extends HTMLElement {
  count = 0;
  @bind
  onclick() { this.count += 1 };
  constructor() {
    this.addEventListener('click', this.onclick);
  }
}

class DoubleCounter extends Counter {
  onclick() { this.count += 2 };
}

You say:

// developers expect this to be the inherited one
// like it is for other scripting languages

Expectations are difficult to assume here. All programming languages are subtly different. As this thread shows there are other languages which do have the same semantics as JS, so it is not as surprising to those developers. When a developers starts using JS one of the things they may need to learn along the way is that JS constructors run purely bottom up and field syntax always defines a property on the instance, only after the parent constructor has finished.

While JavaScript may have 'script' in the name, it is a language used as the primary language for large applications and I would say it has become less dynamic over time to provide some opportunities to optimise it under-the-hood. So comparing it to other statically typed languages does seem appropriate.

[WebReflection]

In my blog post there are already all solutions you mentioned and even more ... but then again, it's about workarounds, not about fulfilling expectations on auto-binding fields at the class level.

My post indeed mention that approach silently flag the class as final without final being a concept in JS, the evil lies there: we are introducing with fields a concept that doesn't exist in JS but exists in the PLs that JS is trying to mimic which makes JS more error prone compared to other scripting PLs where expectations are fulfilled by what developers write and where (as class declaration).

[WebReflection]

@aclaymore

I would say it has become less dynamic over time to provide some opportunities to optimise it under-the-hood

about this, nobody so far has addressed my concerns about leaking fields to parent that returns foreign objects ... no strictly type PL allows that and here we're OK with arbitrary objects receiving fields that don't belong to them ... is this also something OK to explain and expect?

[ljharb]

That any object can have a private field installed on it? Yes, it mirrors the semantics of putting the object in a closed-over weakmap.

[WebReflection]

that a foreign object that doesn't belong at all to the class where either public or private fields are defined suddenly finds itself with those fields that could also side-effect (public) ... see a length field for a class that arbitrarily decides to return an array under some circumstance and it's not preventable at all at user code.

Is this good at PL level? 'cause it's not to me