Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Exports and decorators #7

Open
keithamus opened this issue Mar 24, 2015 · 24 comments
Open

Exports and decorators #7

keithamus opened this issue Mar 24, 2015 · 24 comments

Comments

@keithamus
Copy link

Within a module, which only exports one class, the easiest syntax to use is as follows:

export default class Foo {
}

However, it seems this could be incompatible with class decorators. Would the following work?

@dec
export default class Foo {
}

Or would you have to do:

@dec
class Foo {
}
export default Foo
@nesk
Copy link
Contributor

nesk commented Mar 25, 2015

Good point, the middle code should desugar to this to allow this behaviour:

var Foo = (function () {
  class Foo {
  }

  Foo = dec(Foo) || Foo;
  return Foo;
})();
export default Foo;

@yuchi
Copy link

yuchi commented Mar 25, 2015

AFAIK this:

export default class C {}

already desugars to:

class C {}
export default C;

So this is a non-issue.

Please @sebmck check me on this.

@sebmck
Copy link

sebmck commented Mar 25, 2015

I don't think the grammar specifically allows it but it should be allowed.

@sebmck
Copy link

sebmck commented Mar 25, 2015

The grammar (If I'm reading it correctly) allows you to do:

export default @decorator class Foo {}

@keithamus
Copy link
Author

FYI @sebmck Babel's decorator transform does not allow that. Shall I raise an issue?

@sebmck
Copy link

sebmck commented Mar 25, 2015

@keithamus No it does. I just haven't pushed the latest version to that site.

@keithamus
Copy link
Author

Sweet, many thanks @sebmck! Awesome work!

Back on point - this become a pain point for ES6? export default <thing> seems a much more common paradigm than export default\n <thing>. I would be surprised if I was the only one to trip up on this syntax.

@jayphelps
Copy link
Contributor

It may or may not be worth mentioning that TypeScript's WIP branch for ES6/decorators (demo'd for Angular 2.0) supports decorators before the export declaration. This doesn't appear to be inline with the outlined spec here, but just wanted to note real-world user assumptions.

@decorator export default class Foo {}

Everything else seems the same.

@rbuckton
Copy link

For TypeScript, we've been working with the idea that export and default are modifiers of the ClassDeclaration. As with static in ES6/TypeScript and public, private, protected in TypeScript, all decorators appear before all modifiers. Generally it also looks a bit cleaner if you have decorators on multiple lines in the class declaration:

@decorator
export class C {
  @decorator
  static method() {
  }
}

As opposed to:

export
@decorator
class C {
  @decorator
  static method() {
  }
}

@wycats
Copy link
Owner

wycats commented Mar 25, 2015

@rbuckton the problem with that syntax is that it forecloses the ability to ever decorate exports themselves, no?

@rbuckton
Copy link

@wycats Wouldn't you instead be decorating the declaration that is being exported? I'd rather look into ways of qualifying a decorator in a future proposal. Something similar to how you can disambiguate attributes in C#. An example might be:

@module: moduleDecorator // decorator for module

@export: exportDecorator // decorator for export
@constructor: classDecorator // decorator for class (default)
@classDecorator // decorator for class (default)
export class C {

  @initializer: initializerDecorator // decorator for function of initializer
  @property: propertyDecorator // decorator for property (default)
  @propertyDecorator // decorator for property (default)
  property = 1;

  @return: returnDecorator // decorator for return
  @method: methodDecorator // decorator for function of method
  @property: propertyDecorator // decorator for property (default)
  @propertyDecorator; // decorator for property (default)
  method() {
  }

  @method: methodDecorator // decorator for function of get accessor
  @propertyDecorator // decorator for property (default)
  get accessor() {}

  @method: methodDecorator // decorator for function of set accessor
  set accessor(value) {}

  method2(
    @param: parameterDecorator // decorator for parameter (default)
    @parameterDecorator // decorator for parameter
    p) {
  }
}

Parsing decorator before export doesn't restrict us from future support to decorate exports.

@keithamus
Copy link
Author

@wycats Maybe I'm misunderstanding, but decorating exports over classes sounds a bit like a footgun to me:

@foo
export default class Bar() {}

// lets say the above desugars to this in Node-land:
Bar = foo(module.exports = function Bar() {}) || Bar;

So my Bar class has been exported, and then the exports reference gets decorated - but outside of the assignment, meaning the foo() decorator hasn't applied to the exported class, but my reference internally is now the decorated one - so any work done on Bar after the export will not apply to the exported one, only the internal one.

@sebmck
Copy link

sebmck commented Mar 26, 2015

@keithamus That's not necessarily how it might work since the semantics for it haven't been invented yet. 😄 What @wycats said was that it's problematic to rush into adding additional syntax since it forecloses the ability to do something special with it in the future.

@bathos
Copy link

bathos commented Nov 9, 2015

I'm curious what the current state of this grammar is. In Babel, at present, it is possible to do this:

@decorator export class Foo {}

However I can't find that syntax described in the grammar given with this proposal. Is the github doc out of date, or is the Babel behavior taken from somewhere else?

@ashaffer
Copy link

ashaffer commented Dec 9, 2015

Any thoughts on allowing something like this?

@curry
export * from 'mod'

Where 'curry' would be applied to each element exported by mod.

@trikadin
Copy link

trikadin commented Dec 9, 2015

@bathos, I think that you just didn't notice:

It is also possible to decorate the class itself. In this case, the decorator takes the target constructor.

// A simple decorator
@annotation
class MyClass { }

function annotation(target) {
   // Add a property on target
   target.annotated = true;
}

@trikadin
Copy link

trikadin commented Dec 9, 2015

@ashaffer what's the case?

@ashaffer
Copy link

ashaffer commented Dec 9, 2015

@trikadin You mean what's the use-case? Suppose I have some library of functions and I want to compose them all with some other function, or curry them all, or whatever. Right now I have to do:

import {fn1, fn2, fn3} from 'mod'

fn1 = curry(fn1)
fn2 = curry(fn2)
fn3 = curry(fn3)

export {
  fn1,
  fn2,
  fn3
}

This would be much nicer and equally declarative if I could just indicate that I would like to apply a transformation to each element of mod and re-export it, like this:

@curry
export * from 'mod'

@loganfsmyth
Copy link

@ashaffer Your example is invalid in this case. Imported bindings are immutable, that should throw an exception because you are reassigning them. The core issue with that is that export * from isn't like

const modExports = require('mod');
for (const key in modExports){
    exports[key] = modExports;
}

where you could potentially loop over every property and call the decorator on it. It's more like

const modExports = require('mod');`
for (const key in modExports){
    Object.defineProperty(exports, key, {
        get(){
            return modExports[key];
        }
    });
}

export * from basically says "take every name exposed by mod and expose it as from foo". The value itself isn't even known at that point in time, or may not have even been initialized yet, or it could even be reassigned at a later time. I can't see a way for decorators to fit in there anywhere.

@ashaffer
Copy link

@loganfsmyth That does make sense, I didn't realize they were being exported in that way. However, since exports are immutable, would it not be possible to do something like this?

const modExports = require('mod');
for (const key in modExports){
    const cache = {}
    Object.defineProperty(exports, key, {
        get(){
            return cache.hasOwnProperty(key)
              ? cache[key]
              : (cache[key] = decorator(modExports[key]));
        }
    });
}

In this way, you preserve the late-binding nature of exports that facilitates cyclic dependencies.

@bathos
Copy link

bathos commented Dec 10, 2015

@trikadin, decorating classes isn't what I was referring to. The grammar does clearly cover the example you gave:

ClassDeclaration [Yield, Default] :
    DecoratorList [?Yield]opt class BindingIdentifier [?Yield] ClassTail [?Yield]
    [+Default] DecoratorList [?Yield]opt class ClassTail [?Yield]

However, Babel currently* supports an extension of the ExportDeclaration production that I don't see in the grammar on this proposal -- that's the bit I'm wondering about (@decorator export class Class {}, as opposed to export @decorator class Class {}, which the current grammar does indicate should be possible). For this case -- where DecoratorList may be the initial production of an ExportDeclaration whose Declaration is a ClassDeclaration --we'd need a new variant in the ExportDeclaration production rather than the ClassDeclaration production, as well as clarification as to whether the export-decorator-class form is actually disallowed, since that's what the current grammar says is allowed, as ClassDeclaration may follow export, and ClassDeclaration here is extended to include the case of beginning with DecoratorList. There was some discussion of this earlier in the thread but it seems to have dropped off a while back, and I wasn't sure if there was a concrete resolution.

Short version: which of the following is true?

  1. Babel 5's behavior is officially off-spec, and the only modified production is ClassDeclaration, meaning the current grammar is correct.
  2. Babel 5's behavior is intended to be part of the proposal, and the changes to ExportDeclaration have just not been documented yet.
  3. Same as Unspecified behaviour for distant setters and getters #2, but there is going to be an alternate production of ClassDeclaration for use in ExportDeclaration instead of ClassDeclaration that serves to restrict ExportDeclaration classes to the Babel 5 style, so that there are not then two different ways to decorate an exported class declaration.

(Or: something else entirely.)

* Edit: well, not currently I should say, since decorators aren't covered by Babel 6 at the moment.

@loganfsmyth
Copy link

@ashaffer It's not just late binding though, it's a fully live binding. Exports are immutable from the outside, they are mutable from the inside. For instance a module is perfectly allowed to do the following

export var foo;

foo = class {};

export function reinitialize(){
    foo = class {};
}

Every time the reinitialize function is called, the value of the foo export would change, so for instance

import {foo, reinitialize} from './foo';

var original = foo;

reinitialize();

original !== foo;

@silkentrance
Copy link

@wycats as of babel 6.5.1, the current behavior is that of sigh TypeScript, please close as fixed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests