proposal-class-fields icon indicating copy to clipboard operation
proposal-class-fields copied to clipboard

[[Define]] semantics necessitate workarounds

Open trusktr opened this issue 5 years ago • 7 comments

I had code like the following, which worked great with [[Set]] semantics (f.e. in TypeScript with useDefineForClassFields set to false):

class Sub extends Node {...}
const s = new Sub({prop1: 1, prop2: 2})
class Sub2 extends Sub {...}
const s2 = new Sub2({prop1: 1, prop2: 2, prop3: 3})

but due to [[Define]] semantics, I had to change it to

class Sub extends Node {...}
const s = new Sub().set({prop1: 1, prop2: 2})
class Sub2 extends Sub {...}
const s2 = new Sub2().set({prop1: 1, prop2: 2, prop3: 3})

because of [[Define]]'s accessor-overriding nature.

I like the new set() API better (because it duals as an API for setting properties at any time and via constructor, not just via constructor), but the main point is I was forced to make this change, whereas otherwise I could've opted into making this API later (and it would work regardless of [[Set]] vs [[Define]]) and avoided making a breaking change.

trusktr avatar Sep 30 '20 18:09 trusktr

I'm not sure why this change was forced; you could have used this.a = prop1 in the constructor instead of a class field and it'd have used Set semantics.

ljharb avatar Sep 30 '20 18:09 ljharb

Changing from foo = 123 to this.foo = 123 in pre-existing code is a forced change.

The pre-existing code also uses decorators, and ergonomics would be lost in moving things into the constructor. For big classes, this would mean even more refactoring (where private properties are co-located with methods).

trusktr avatar Sep 30 '20 18:09 trusktr

I agree it's a forced change - but it's a forced change within your class that does not cause a breaking change, and it's only caused by using pre-stage 3 TC39 proposals.

ljharb avatar Sep 30 '20 18:09 ljharb

it's a forced change within your class that does not cause a breaking change

Sure, I can change how my class is implemented, so that people instantiating my class get the same API without a breaking change.

But there's consumer code that creates subclasses following the same patterns, and they need to make a change too.

For example, suppose the above Sub class is in a library A, and Sub2 is consumer code in library B that wishes to provide the same usage patterns for a hypothetical Sub3 class in yet another library or app. Now the author of library B experiences a breaking change in library A which has modified the method by which classes need to be defined. And then that propagates to library C, especially if those authors would like to still support both [[Set]] and [[Define]] semantics (for new code, and for old code still running in Babel loose more or TypeScript without useDefineForClassFields).

trusktr avatar Sep 30 '20 18:09 trusktr

Updated my previous comment to make it more clear. There does not need to be a breaking in how people use instances of those classes, but there does need to be breaking changes in how consumers define new subclasses.

trusktr avatar Oct 06 '20 00:10 trusktr

Another thing is that to fix various libraries that previously always worked fine (with [[Set]] semantics), and we wish to avoiding re-writing the implementation, we have to do annoying things like this:

Before:

export class ConsumerClass extends LibraryClass {
  // ... before class fields, there was only [[Set]] in here, unless someone use `Object.defineProperty` but that's considered a clear indicator of overriding base behavior class.
}

Now:

export const ConsumerClass = LibraryMixin(class ConsumerClass {
  // ... before class fields, there was only [[Set]] in here, unless someone use `Object.defineProperty` but that's considered a clear indicator of overriding base behavior class.
})

And the reason for such a change is so that the library code can run after fields are [[Define]]ed.

This whole thing is such a mess, and who knows when decorators are coming out, and specs should not break ecosystems for very insignificant gains.

trusktr avatar Oct 06 '20 00:10 trusktr

Most comments that I post in this GitHub issue tracker happen indirectly because I encountered an issue in real-world code and then came here to post a comment. It's just true that [[Define]] has caused various headaches.

I can avoid ever using class fields, but I can't force consumers of all my code to do that, so things still won't work one way even if I want it to.

trusktr avatar Oct 06 '20 00:10 trusktr