search

rdking / proposal-class-members

https://zenparsing.github.io/js-classes-1.1/
7 stars 0 forks source link

Babel plugin #9

Open mbrowne opened 5 years ago

mbrowne commented 5 years ago

I created a preliminary Babel plugin for this proposal. I probably seem like an unlikely person to create this plugin, but although I still favor the class fields proposal I do think this proposal deserves at least a chance of becoming better known by the community. I think being able to actually try it out like we now can with class fields is an important part of that. (And also, to be honest, this was a good opportunity to update my Babel plugin authoring skills to Babel 7 ;-) ).

Currently it only transpiles private variables, including private static variables (AKA class variables). I wrote some code to transform public properties as well and put them on the prototype as per @rdking's desire, but I commented it out since that part seems to still be evolving. Also, I think it would be disastrous to simply put properties on the prototype without doing something to address the foot-gun, so I didn't want to just put public properties on the prototype and leave it at that. (However well-known the foot-gun might be outside of classes, putting data properties on the prototype would be surprising enough to many developers in the first place that expecting people to be aware of the foot-gun is completely unrealistic IMO.)

I realize that if the plugin doesn't implement this proposal correctly, it could do more harm than good, but I also had to consider what makes the most sense for transpilation purposes, so I settled on WeakMaps as with @babel/plugin-proposal-class-properties (which implements the class fields proposal, and which I used as a reference for the implementation).

My goal here was to get a basic POC working...I'm not really interested in updating it with all the features in the proposal. @rdking, I don't know if you would have time or interest in working on it, but if not then maybe one of the other supporters of this proposal or classes 1.1 would be interested.

A note on the implementation: the only way to add new syntax to Babel is to fork the parser. Unfortunately the only way I was able to get it completely working was to use yarn's resolutions feature because I also needed to fork @babel/types, which needed to make parsing-related helper methods work. Because of that and other reasons, using the existing examples directory is a lot easier than creating your own project. Hopefully I'll get a response to this issue: https://github.com/babel/babel/issues/9009.

CC @hax @shannon @Igmat @bdistin @aimingoo

mbrowne commented 4 years ago

The helpers are definitely runtime.

I realize that...I'm not talking about making them compile-time, but rather how they get included when transpiling with Babel. In inline mode (the default), they'll be included many times (at the top of every file that uses classes I believe). But in runtime mode, they'll only exist in one place and will be imported or required wherever they're needed. That's why I recommend using Babel's infrastructure for this rather than just including them at the top of every class declaration (which might have been your plan to begin with, I just wanted to make a clear recommendation on this). For an example, see https://github.com/mbrowne/babel-plugin-class-members/blob/master/packages/babel-plugin-proposal-class-members/src/addHelpers.js and https://github.com/mbrowne/babel-plugin-class-members/blob/master/packages/babel-plugin-proposal-class-members/src/index.js#L131.

Of course there's a balance between bundle size and runtime performance, so use your best judgment on how much to rely on helpers to reduce code size...your latest strict mode version looks good at a glance.

Something else I noticed: I see that you're using a single WeakMap for all instance properties. I didn't look at your logic that closely, but in any case I would test it to make sure it's not affected by the "pitfall" mentioned here: https://github.com/tc39/proposal-class-fields/blob/master/PRIVATE_SYNTAX_FAQ.md#how-can-you-model-encapsulation-using-weakmaps. That pitfall is presumably the reason why Babel's class-properties transform uses a separate WeakMap for each field. No need to explain all the details if you've already accounted for this, I just wanted to mention it.

P.S. Note that there could be legitimate use cases for wanting to use call or apply to set a custom target when calling a method that exists on a property value. So I think using a separate WeakMap for each field might be the safest approach, rather than trying to do any auto-binding (not that you're currently doing any auto-binding)...

rdking commented 4 years ago

I'm not talking about making them compile-time, but rather how they get included when transpiling with Babel.

Sorry, I misunderstood. I intend to use the require approach. Copy & paste doesn't feel like a good idea ever.

...I would test it to make sure it's not affected by the "pitfall" mentioned here...

That's not a pitfall as much as it is a coding error. I've already got this one covered. In case you want to know, the fix is to wrap the private function to ensure it is called correctly. It's what I think of as one of the few good uses for a strict-mode function.