angular / tsickle

Tsickle — TypeScript to Closure Translator
MIT License
896 stars 111 forks source link

Error on @export on a class #249

Open evmar opened 7 years ago

evmar commented 7 years ago

This has become an error in the Closure compiler, so we should match.

MatrixFrog commented 7 years ago

@export is allowed on classes, just not inside of goog.modules. I added some new docs for this internally and I think the library team is working on externalizing them as well.

evmar commented 7 years ago

@MatrixFrog tsickle always generates goog.modules (to match ES module semantics) so I think we should just never allow it (?). Not sure I followed you though.

MatrixFrog commented 7 years ago

I'm just saying it has nothing to do with classes (as opposed to functions). But I forgot that tsickle output is always a goog.module. So yes, just ban @export entirely I guess, and maybe have an error message that points users toward the docs (which don't seem to be available outside Google yet but I'm hoping they will be soon).

mprobst commented 7 years ago

Careful: @export is commonly needed on fields and methods in Angular 1 code. E.g.:

class TodoCtrl {
  /** @export */ fieldThatsUsedInTemplate = 'hello';
}

So we cannot ban them generally, we could only ban them on top level symbols (or whatever the heuristic is there?).

Martin

MatrixFrog commented 7 years ago

Yes, banning them on module-level symbols would be the thing to do then, I think. Thanks!