Implement new decorator proposal when finalized #2645

Open
kittens opened this Issue Oct 30, 2015 · 11 comments

Projects

None yet

8 participants

@kittens
Member
kittens commented Oct 30, 2015

No description provided.

@SrMouraSilva

Decorating a method

Babel 6.0.13

//.babelrc
{
  "plugins": [
      "syntax-decorators",
      "transform-decorators"
  ]
}

Original

function tester(a, b, c) {
    return c;
}

class Test {
    @tester
    test(a) {
        return a;
    }
}

Babelled

function tester(a, b, c) {
    return c;
}

class Test {}
Test;

@SrMouraSilva

Decorating a class (with class)

Babel 6.0.13

//.babelrc
{
  "plugins": [
      "syntax-decorators",
      "transform-decorators"
  ]
}
function classTester(target) {

}

function tester(a, b, c) {
    return c;
}

@classTester
class Test {
    @tester
    test(a) {
        return a;
    }
}
D:\Mateus\#evento\servidor\app\protocolo>babel ProtocoloComunicacao.js >> test.j
s
TypeError: ProtocoloComunicacao.js: Property right of AssignmentExpression expec
ted node to be of a type ["Expression"] but instead got "Decorator"
    at Object.validate (D:\Mateus\#evento\servidor\node_modules\babel-plugin-tra
nsform-decorators\node_modules\babel-types\lib\definitions\index.js:97:13)
    at validate (D:\Mateus\#evento\servidor\node_modules\babel-plugin-transform-
decorators\node_modules\babel-types\lib\index.js:267:9)
    at Object.builder (D:\Mateus\#evento\servidor\node_modules\babel-plugin-tran
sform-decorators\node_modules\babel-types\lib\index.js:243:7)
    at maybeMemoise (D:\Mateus\#evento\servidor\node_modules\babel-plugin-transf
orm-decorators\node_modules\babel-helper-explode-class\lib\index.js:29:32)
    at memoiseDecorators (D:\Mateus\#evento\servidor\node_modules\babel-plugin-t
ransform-decorators\node_modules\babel-helper-explode-class\lib\index.js:56:7)
    at Object.exports.default (D:\Mateus\#evento\servidor\node_modules\babel-plu
gin-transform-decorators\node_modules\babel-helper-explode-class\lib\index.js:61
:3)
    at PluginPass.ClassDeclaration (D:\Mateus\#evento\servidor\node_modules\babe
l-plugin-transform-decorators\lib\index.js:164:45)
    at newFn (C:\Users\JoséAntônio\AppData\Roaming\npm\node_modules\babel-cli\no
de_modules\babel-core\node_modules\babel-traverse\lib\visitors.js:233:27)
    at NodePath._call (C:\Users\JoséAntônio\AppData\Roaming\npm\node_modules\bab
el-cli\node_modules\babel-core\node_modules\babel-traverse\lib\path\context.js:7
2:18)
    at NodePath.call (C:\Users\JoséAntônio\AppData\Roaming\npm\node_modules\babe
l-cli\node_modules\babel-core\node_modules\babel-traverse\lib\path\context.js:44
:17)
@SrMouraSilva

For methods:

In babel-plugin-transform-decorators

Line 41: Remove the decorator methods

Line 47: ???

I don't found a similar code in babel 5.x :/

@kittens kittens locked and limited conversation to collaborators Oct 30, 2015
@kittens
Member
kittens commented Oct 30, 2015

The Babel issue tracker is not for support queries.

@hzoo hzoo added the bug label Nov 7, 2015
@hzoo hzoo removed the failing test label Feb 8, 2016
@babel-bot
Collaborator

Comment originally made by @SrMouraSilva on 2015-11-20T00:36:29.000Z

https://github.com/babel/babel/blob/master/CHANGELOG.md#6119

6.1.19

Bug Fix

  • Add config check to package.json babel reading.
  • Fix source maps merging.
  • Ignore callee supers when doing spread compilation
  • Polish
  • Add hard error when we see decorators.

Comment originally made by @kittens on 2015-11-21T03:34:20.000Z

A lot of people are subscribing to this (thank you Phabricator!) so I figure I'll give some more context and an update.

Decorators were removed in Babel 6 because the decorator proposal has vastly changed from the version that's in Babel 5. I could implement the old proposal in Babel 6 which would make existing Babel 5 decorator code work but when I updated the transformer for the new version of the proposal, your code would break and I'd be violating semver. I'm not willing to do another major bump either as majors in a project like Babel are extremely difficult to coordinate as there's so many integrations using it.


Comment originally made by @ide on 2015-11-21T03:46:03.000Z

@sebmck would you be open to publishing a nonkosher version of the plugin -- for example, transform-babel-5-decorators -- so that existing consumers of Babel have an upgrade path without waiting for TC39?

The major use case I'm familiar with is React Native, which is shipping with Babel 6 shortly (the next release candidate is going out in the next few days). RN core doesn't use Babel 5 decorators but many npm packages in the ecosystem do because RN's packager encourages adoption of ES2015 and beyond by making it easy to add Babel 5 flags / Babel 6 plugins.

All this is to say that temporarily supporting old decorators in Babel 6 in a way that doesn't break semver will ease the transition to Babel 6 for folks using decorators.


Comment originally made by @kittens on 2015-11-21T05:12:11.000Z

@ide It's just a lot of work. I can look into doing a straight port of the Babel 5 classes plugin and releasing it as a plugin though as that should be fairly easy which I guess is basically what you're asking.


Comment originally made by @ide on 2015-11-21T06:48:25.000Z

@sebmck a port of the old plugin is exactly what I think would help Babel 5 users migrate a codebase with (old) decorators to use Babel 6. I don't want this to become a pain for you so if it's easy to port the old plugin that's great. Otherwise I can look into porting it myself but I think you'd be much faster at it.


Comment originally made by @zerkalica on 2015-11-21T09:23:12.000Z

Where is decorators proposal changed, where issues, discuss about it? I try to google it, but i can't find anything, except old proposal: https://github.com/wycats/javascript-decorators


Comment originally made by @rthewhite on 2015-11-23T07:51:34.000Z

! In T2645#65832, @zerkalica wrote:
Where is decorators proposal changed, where issues, discuss about it? I try to google it, but i can't find anything, except old proposal: https://github.com/wycats/javascript-decorators

Kinda interested in this as well, what has changed?


Comment originally made by @epsitec on 2015-11-23T07:54:37.000Z

@sebmck I'd be happy to sponsor a quick port of the Babel 5 classes plugin to Babel 6.2, just to get tons of projects using @xxx to continue to work with Babel 6.


Comment originally made by @elliottsj on 2015-11-29T22:15:21.000Z

! In T2645#65832, @zerkalica wrote:
Where is decorators proposal changed, where issues, discuss about it? I try to google it, but i can't find anything, except old proposal: https://github.com/wycats/javascript-decorators

If I'm not mistaken, these are the changes to the decorator proposal to which @sebmck is referring:
wycats/javascript-decorators#36

cc @rthewhite


Comment originally made by @ide on 2015-11-30T19:10:44.000Z

I'm going to work on porting the Babel 5 decorators plugin to Babel 6 since there's high demand for it. If you have experience implementing Babel plugins and want to help out find me on Reactiflux (keep this Phabricator task free of noise).


Comment originally made by @kmalakoff on 2015-12-02T05:57:27.000Z

@ide if you are porting the Babel 5 decorators to Babel 6 as a stop gap, I collected some links to resources and problems to do so in the other task (https://phabricator.babeljs.io/T2702) when I was considering doing it myself.

Finger in the air estimate...it is probably 1 to 3 days of work to implement and port all of the tests given the complexity.


Comment originally made by @ide on 2015-12-02T06:26:17.000Z

@kmalakoff I started working on it but I'm probably not going to get it done quickly because as you mentioned the learning curve is high for writing a significant plugin like this one.

Porting the Babel 5 functionality is definitely what I would put my support behind because it facilitates the upgrade to Babel 6. A migration plan from Babel 5 --> Babel 6 + old decorators --> Babel 6 + new decorators is probably best for the community and it lets people upgrade to Babel 6 while authors of decorator libraries work on updating their packages. Plus this week's release of React Native will ship with Babel 6 so several early adopters aren't going to be able to test the latest RN on projects using decorators.

I put up an airpair request for help with the plugin but I'd rather give $300 to someone who does a good job writing the plugin since I think it'd be faster that way. It's probably less than you'd be paid if formally contracting to write it but you'd be doing it first and foremost for open source and the external community.


Comment originally made by @loganfsmyth on 2015-12-03T06:41:53.000Z

Hey everyone, I just want to say that we're well aware of how much of a pain the lack of decorators in 6.x has been. Unfortunately, things have taken a bit longer than originally hoped. Rather than try to have Babel 6 support both decorator proposals, the intention is still for Babel 6 to implement the new proposal once it has been officially presented to TC39.

That said, your comments have definitely been heard, so I've put together a plugin that does its best to implement Babel 5 style decorators in Babel 6: https://www.npmjs.com/package/babel-plugin-transform-decorators-legacy

One important limitation, which is noted in the README, is that the decorators on class properties can only mutate the initializer itself, not enumerable, writable, and configurable, which is not in line with Babel 5's behavior, but is realistically probably not something that is used in the common usecases.


Comment originally made by @m4n3z40 on 2015-12-03T21:50:25.000Z

@loganfsmyth Thanks for the plugin, man. It works awesome.

But there's a little gotcha, if you're using the "transform-class-properties" in your project as well, make sure to put it BEFORE the "transform-decorators-legacy" value in your .babelrc plugins configuration. If you don't do that the "transform-class-properties" plugin gets lost and introduces a undefined variable, which creates an error ("_undefined is not defined").

But again, many thanks!


Comment originally made by @loganfsmyth on 2015-12-04T02:21:33.000Z

@m4n3z40 Sounds like https://phabricator.babeljs.io/T2983 for the _undefined issue, which should be fixed in the next release.

Not 100% on the ordering, essentially if you want to be able to decorate class properties the decorator plugin should be run before the class property plugin, though perhaps that requires the fix mentioned above.


Comment originally made by @kmalakoff on 2015-12-04T04:14:14.000Z

@loganfsmyth thank you so much for this. I copied the code from the issue you mentioned and the _undefined was fixed.

I'm not totally sure precisely on what this entails: the decorators on class properties can only mutate the initializer itself, not enumerable, writable, and configurable, but I'm using https://mweststrate.github.io/mobservable/refguide/observable-decorator.html and the property decorators for @observable are not showing up.

I'm assuming this falls outside the support target or is this a bug?


Comment originally made by @ajwhite on 2015-12-05T03:58:10.000Z

HUGE thanks to @loganfsmyth for putting this together. I was hesitant from upgrading to React Native 0.16 to lose all my decorators.


Comment originally made by @loganfsmyth on 2015-12-15T17:42:21.000Z

I've fixed the limitations I originally mentioned about https://www.npmjs.com/package/babel-plugin-transform-decorators-legacy and I'd now expect decorators to work pretty much the same as they did in Babel 5 if you use that transform.


Comment originally made by @loganfsmyth on 2015-12-16T03:44:21.000Z

Lowering the priority on this given that it's not a high priority since we don't even have a spec to build yet, and the legacy plugin should alleviate most of the rush.


Comment originally made by @calebmer on 2015-12-16T03:53:33.000Z

Does the legacy plugin work with Babel 6? If so, which one is it?


Comment originally made by @loganfsmyth on 2015-12-16T04:17:43.000Z

Yes, it is a Babel 6 plugin. I'm not sure what you mean by "which one". I linked to it above, you'd install it like any other Babel plugin.


Comment originally made by @kmalakoff on 2015-12-18T16:23:59.000Z

@loganfsmyth I tried on the day of the latest release a few days ago, but had to stop because I couldn't get it to work with mobservable. I'm trying again today and trying to track down the problems....

First problem: Uncaught Error: Decorating class property failed. Please ensure that transform-class-properties is enabled my .babelrc has stage-1 enabled:

{
  presets: ['es2015', 'stage-1', 'react'],
  plugins: ['add-module-exports',  'transform-decorators-legacy']
}

If I manually add transform-class-properties (maybe I'm configuring incorrectly?) I do not get the warning:

{
  presets: ['es2015', 'stage-1', 'react'],
  plugins: ['add-module-exports', 'transform-class-properties', 'transform-decorators-legacy']
}

but I run into a second problem:

class MyClass {
  @observable hello = {things: []};

  @observable get world() { return this.hello.things; }
}

The world decorator is added, but the hello one is missing from the class decoration section. So it looks like there is a change / bug in how decorators handle class properties...(other more simple ones like @observable hello = null; are skipped too)


Comment originally made by @AshCoolman on 2015-12-19T06:51:38.000Z

! In T2645#67560, @loganfsmyth wrote:
Rather than try to have Babel 6 support both decorator proposals, the intention is still for Babel 6 to implement the new proposal once it has been officially presented to TC39.

First thanks for the legacy plugin. Secondly, do you know if the official presentation has a date? (or is attached to some known milestone)?


Comment originally made by @loganfsmyth on 2015-12-19T18:32:22.000Z

@kmalakoff Could you report your issue on the github project for decorators-legacy so we don't spam everyone subscribed to this issue with unrelated stuff?

@AshCoolman I don't know the specifics, sorry. I think it'll essentially be "when wycats feels the proposal is ready".


Comment originally made by @kmalakoff on 2015-12-19T21:22:46.000Z

@loganfsmyth done (loganfsmyth/babel-plugin-transform-decorators-legacy#13).

You bring up a good point...people will who subscribed to this issue when it was called "Fix decorators" (or who were merged in from the other issue related to decorator support which is where I was added from: https://phabricator.babeljs.io/T2702) would be very interested in knowing about potential bugs....I have probably spent cumulatively 16 hours upgrading to and reverting from Babel 6 due to decorator support / issues...you can see that I really want to upgrade!

My thinking on decorators and this issue...I assume that the new proposal could be stuck in committee for a long time and I need to just find a pragmatic solution to migrating to Babel 6 (I'm still on Babel 5 due to decorator issues). I've been watching this issue for updates since I got here from where you pointed me from Github (started: 69fb185#commitcomment-14865033, then https://phabricator.babeljs.io/T2702, then here, and now being directed here: https://github.com/loganfsmyth/babel-plugin-transform-decorators-legacy).

I would love a single place to get updates on legacy compatibility issues which I assumed (until you just mentioned it) that it was here...

Maybe it would be better to open a new issue called "Implement new decorator proposal when finalized" and keep this issue focussed on "Fix decorators" (I'm not interested in tracking "Implement new decorator proposal when finalized"). Alternatively, you could create an issue called "Fix decorators" (I'd be interested in knowing about any other issues people encounter) or make it official that this issue is now only for the future decorator spec and people who are not interested in it should unsubscribe (which is sounds like this issue has morphed into).

Sorry for the verbose response, just trying to make things totally clear since I'm assuming I'm not the only person who might have been merged into this issue which may not quite be a match for what we are looking for...based on the original purpose of helping people who are using decorators migrate to Babel 6, I suspect most people are here for current support of decorators in Babel 6 instead of the future spec...but I could be wrong...

I really appreciate you carrying the torch on this!


Comment originally made by @loganfsmyth on 2015-12-20T19:02:11.000Z

If you'd like to track issues with decorators-legacy, watching that github repo would be the best option. Since it's not part of the main Babel package, I think it would just clutter things up in this issue tracker to have an issue about it here.

I'd recommend that people following this issue for the "Fix decorators" rather than the "Reimplement decorator proposal" case unsubscribe since that should now be fully addressed by decorators-legacy.


Comment originally made by @Download on 2016-01-26T10:19:40.000Z

@loganfsmyth
Thanks, I am using your plugin with Babel 6 and my decorators are working again!

@Babel team
I love your project! Thank you so much for giving this to the community... BUT
I think you guys messed up on this one. You broke the builds of everyone that was using decorators and upgraded to Babel 6.... And for what exactly? Yes, the spec is still in flux, but the same thing could be said for most of the stuff out there in stage 1. Also, even though the proposal may have changed, it seems for the more basic uses of decorators this does not actually have much impact at all... Unless I am reading the current proposal all wrong it seems that all my code will work unchanged when Babel 6 again adds support for decorators.

I think I good old warning, backed by a well-written issue / blog post explaining the problems would have been a much, much better choice.


Comment originally made by @silkentrance on 2016-01-28T14:20:14.000Z

@Download it is not so much the features that have been thrown out/rescheduled until the spec gets fixed, but rather the obnoxious smaller issues with that 6.x+ release. I for my part think that they were just perfload testing the phabricator instance and the community 😁


Comment originally made by @loganfsmyth on 2016-01-28T16:46:18.000Z

@silkentrance @Download There are certainly other higher-priority tasks, but my previous statements are not incorrect, we are indeed waiting on the spec to stabilize. I will definitely admit that it has taken longer for that to happen then any of us were expecting, which is why we decided to go forward with the -legacy plugin.

@Download If you are referring to https://github.com/wycats/javascript-decorators/blob/master/README.md, mind the note at the top:

This file is under active development. Refer to interop/reusability.md for the most up to date description.

The in-progress decorator specification in https://github.com/wycats/javascript-decorators/blob/master/interop/reusability.md is the newer one and is quite different from the old one. Existing decorators would not work unchanged.


Comment originally made by @mattinsler on 2016-02-01T17:09:33.000Z

Turning on transform-decorators-legacy will cause errors if you use class properties in a specific way.

class Foo {
  static SECOND = 1000;
  static MINUTE = Foo.SECOND * 60;
}

This will say that Foo is undefined while trying to evaluate 'Foo.SECOND'. Without the decorators-legacy plugin this works.


Comment originally made by @loganfsmyth on 2016-02-01T17:30:18.000Z

@mattinsler This thread is about the implementation of babel-plugin-transform-decorators. babel-plugin-transform-decorators-legacy lives in https://github.com/loganfsmyth/babel-plugin-transform-decorators-legacy.


Comment originally made by @callumlocke on 2016-03-20T22:00:12.000Z

The v6 docs say that decorators are supported, but they're not. A few words about this in the docs would have saved me about 3 hours of wasted time today. Please can someone merge this PR to update the docs: babel/babel.github.io#765


Comment originally made by @isiahmeadows on 2016-07-26T10:21:50.000Z

I've seen recent activity in the new-ish TC39 repo, so maybe this might stabilize soon and be re-implemented?

By the way, it's going to require a major overhaul for most decorators, because the interfaces are very different. Note that the interfaces are just that: duck-typed interfaces. The descriptors are plain objects, not actual class instances.

=== Property/Method Decorators ===

Property and method decorators don't accept the descriptor directly, but in a wrapper object now. They accept a MemberDescriptor and return a MemberDescriptor.

lang=ts
type PropertyDecorator = (descriptor: MemberDescriptor) => MemberDescriptor;

interface MemberDesciptor {
  kind: "property";
  key: string | symbol;
  isStatic: boolean;
  descriptor: PropertyDescriptor;
  extras?: MemberDescriptor[];
  finisher?: (klass) => void;
}
  • kind is required to be "property", but is there to open up the possibility of other kinds.
  • key is the object key.
  • isStatic is whether the descriptor is a static property or not.
  • descriptor is the incoming property's descriptor
  • extras are an optional list of extra descriptors to add to the object, and it's unset coming in.
  • finisher is an optional finisher to have called with the final instance at the end.

=== Class Decorators ===

Class decorators no longer take the class itself, but a descriptor consisting of the class, its parent class (i.e. `extends Parent) if one exists, and a set of MemberDescriptors on the class.

lang=ts
type ClassDecorator = (descriptor: ClassDescriptor) => ClassDescriptor

interface ClassDescriptor {
  klass: Function;
  parent: Function | undefined;
  members: MemberDescriptor[];
}
  • klass is the class being decorated.
  • parent is the class it extends, if any.
  • members are the MemberDecorators representing the static and instance properties/methods of the class.

And yes, Angular, Aurelia, and many others will need to be updated for this. (They may end up at least temporarily supporting both during the transition.) TypeScript can always make the types union types instead, to ease their transition. But regardless, it'll break a lot of things, simply because quite a few people jumped the gun and embraced decorators a little too early.


Comment originally made by @antipin on 2016-07-27T08:05:00.000Z

Thanks for info @isiahmeadows

Am I got it right, there is no access to class itself in property/method decorator now?


Comment originally made by @addyosmani on 2016-07-29T00:17:54.000Z

From the looks of it, class and property decorators have advanced to stage 2 at TC39: tc39/proposals@97eb62f


Comment originally made by Mark Lundin (mark) on 2016-07-29T06:09:13.000Z

Fantastic, I guess this must have been approved in the TC39 meeting this week.

Can this now be safely updated in Babel?


Comment originally made by @jkrems on 2016-07-31T20:03:23.000Z

I started working on an initial implementation (mostly for even understanding what the new proposal does / allows) here: https://github.com/jkrems/babel-plugin-transform-decorators-stage-2-initial. Very early stage but might be helpful.

@babel-bot babel-bot changed the title from Fix decorators to Implement new decorator proposal when finalized Sep 7, 2016
@hzoo hzoo added the es-proposal label Oct 10, 2016
@hzoo hzoo unlocked this conversation Oct 10, 2016
@idchlife

Any info when Decorators will be again available in babel?

@hzoo
Member
hzoo commented Oct 12, 2016

If we have an update we'll make a comment here - I don't think anyone has time to work on it

@just-boris

@idchlife I'd like to mention that this is not about working on actual Babel plugin or something.
There needs to be done some work on the spec.

There were some questions left, the last update was on July meeting.

For the time being, there is an external plugin babel-plugin-transform-decorators-legacy. The thing why it doesn't belong to core repo, because it likely will change, even the @-symbol might be changed

@chicoxyzzy
Contributor

Recently Yehuda said that proposal is waiting for implementations. Symbol unlikely will be changed because a huge part of committee is opposed to sigil swap with private properties. So IMO it's best time to start hacking on this right now :)

@DrewML
Member
DrewML commented Dec 7, 2016 edited

I've started working on a new plugin for Babylon to implement the updated grammar for the Stage 3 version of the proposal. Once that is done, I can start hacking on the transform.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment