✨ Move param dec to class

[wtho]

The parameter decorator is now not applied on the class after the
class declaration, but instead added as class decorator itself.

This is imitated from TypeScript compilation.
It leads to more consistency between compilers and increases
compatibility of libraries that depend on this behavior.

Example

@Injectable()
export class Decoratee {
  constructor(@Inject() param1: String) { }
}

Compilation using TSC

let Decoratee = class Decoratee {
    constructor(token) { }
};
Decoratee = tslib_1.__decorate([
    Injectable(),
    tslib_1.__param(0, Inject()),
    tslib_1.__metadata("design:paramtypes", [String])
], Decoratee);
exports.Decoratee = Decoratee;

To understand the change in this PR, we should also have a look at tslib.__param:

__param = function (paramIndex, decorator) {
  return function (target, key) { decorator(target, key, paramIndex); }
};

So looking at tslib_1.__param(idx, Inject()) and knowing that Inject() will return a decorator function with the two arguments target and function, the __param call basically is the decorator

function(target, key) { return (0, Inject)()(target, key, idx) }

Compilation using babel and this plugin, before this change

var Decoratee = (
  _dec = (0, Injectable)(),
  _dec2 = Reflect.metadata("design:paramtypes", [String]),
  _dec(_class = _dec2(_class = function Decoratee(token) {
    _classCallCheck(this, Decoratee);
  }) || _class) || _class);
exports.Decoratee = Decoratee;
(0, Inject)()(target, undefined, 0);

Compilation using babel with this PR change

var Decoratee = (
  _dec = (0, Injectable)(),
  _dec2 = function _dec2(target) { return (0, Inject)()(target, undefined, 0); },
  _dec3 = Reflect.metadata("design:paramtypes", [String]),
  _dec(_class = _dec2(_class = _dec3(_class = function Decoratee(token) {
    _classCallCheck(this, Decoratee);
  }) || _class) || _class) || _class);
exports.Decoratee = Decoratee;

As Decorators are applied in reverse order (in the example above dec3, then dec2, and then dec), some libraries may rely on the class decorator (here dec) being applied after the parameter decorator (dec2). Angular's Dependency Injection in fact does rely on this. This PR solves this.

How it works

The new function in parameterVisitor transforms the previously postponed decorator call into a classical decorator, that gets called with the others. It wraps the expression the same way tslib.__param does inside another function call, as it has to be passed previously.

Side Effects

If the class has only a parameter decorator, no class decorator, previously Reflect.metadat('design:paramtypes', ... was never called on the class. Now that we transform the parameter decorator to a kind of class decorator, it will be called anyway. This leads to the novel possibility of metadata about constructor parameter being available through reflection at runtime, although no true class decorator is applied. This coincidentally solves #22.

To Clarify

• Does this break other DI libraries? Should be checked!
• Do we need additional tests? Maybe one where the decorator order is assured

TODO

• I regenerated the snapshots - please review them thoroughly
• TS also handles parameter decorator on methods differently - see comment below

Solves #23, #22

[codecov]

Codecov Report

Merging #24 into master will decrease coverage by 0.36%.
The diff coverage is 92.85%.

@@            Coverage Diff             @@
##           master      #24      +/-   ##
==========================================
- Coverage   91.89%   91.52%   -0.37%     
==========================================
  Files           4        4              
  Lines         111      118       +7     
  Branches       41       45       +4     
==========================================
+ Hits          102      108       +6     
- Misses          9       10       +1

Impacted Files	Coverage Δ
src/metadata/metadataVisitor.ts	93.75% <100%> (+0.89%)	⬆️
src/parameter/parameterVisitor.ts	95.83% <90.9%> (-4.17%)	⬇️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8ab68d7...1371f6b. Read the comment docs.

[wtho]

Another compilation divergence came accross:

// example class

class Decoratee {
  someMethod(@Inject() param: String) {}
}

Compiled by TS

class Decoratee {
    someMethod(param) { }
}
tslib_1.__decorate([
    tslib_1.__param(0, Inject()),
    tslib_1.__metadata("design:type", Function),
    tslib_1.__metadata("design:paramtypes", [String]),
    tslib_1.__metadata("design:returntype", void 0)
], Decoratee.prototype, "someMethod", null);
exports.Decoratee = Decoratee;

Compiled by babel

var Decoratee =
/*#__PURE__*/
function () {
  function Decoratee() {
    _classCallCheck(this, Decoratee);
  }
  _createClass(Decoratee, [{
    key: "someMethod",
    value: function someMethod(param) {}
  }]);
  return Decoratee;
}();
exports.Decoratee = Decoratee;
(0, Inject)()(Decoratee.prototype, "someMethod", 0);

TS adds more metadata. Should we handle this in this PR as well?

---

Another question: Can I change

babel-plugin-transform-typescript-metadata/src/metadata/serializeType.ts

Line 1 in 8ab68d7

import * as t from '@babel/types';

to import { types as t } from '@babel/core'; so that the CI passes? - I see, you just added it in the last commit for a reason.

[wtho]

So I implemented the decorator installation anyway (by decorator installation I mean placing the decorator in the class/field decorator array instead of adding it after the class).

I extended it quite a bit by now, so please have a good look and tell me if I should split it in several PRs or if you want something changed. I am quite excited that babel comes this close to TypeScript with just some wrapping.

Metadata on Methods (other than the constructor)

I added the design:type decorator to methods as well, I think there is no reason we do not install it.

The design:returntype decorator on the other hand seems to be trickier, TypeScript gets the node type during compilation, whereas babel does not analyze it. We would have to do some quite complicated work, so I think this is not required until someone requests it. I still left it in for, commented out.

[wtho]

Hey @leonardfactory, can you have a look at this PR? Is there anything I should change?

[leonardfactory]

Hi @wtho! sorry for the delay but I've been busy these days. Your job seems outstanding. I'm reviewing it but I'll need some time to do some tests and check it 👍

[leonardfactory]

You got a real good catch here! Thank you for this contribution, and for your precious work. It seems to me this should work just as usual and should not break existing projects, however just adapt the code style in order to keep it clean. Waiting for your changes in order to merge this cool piece of work.

[leonardfactory]

Please, add a comment here warning that following code is just an hint for future works about returnType: elseway this seems just like a forgotten copy paste 😄

[wtho]

I added a shorter comment with a useful reference to the TSC compiler. If someone wants to tackle this at some time, this will be a good starting point.

[wtho]

and should not break existing projects

I think it won't behave differently in most projects, although some might depend on the way or order the decorators are applied. I personally would prefer to release a major version, but it's up to you.

If you want me to add additional notes to the README or changelog, I'll be happy to do so!

[wtho]

@leonardfactory we are still waiting on this feature. Would be awesome if you could have another look at it.

I already fixed the changes requested, and thinking again about it being breaking, I think it does not have to be. It adds some additional decorators, but does not remove any that were applied before.

If you are hesitant to merge and deploy, I would be happy if you could already publish a beta version.

Cheers!

[codecov-io]

Codecov Report

Merging #24 into master will decrease coverage by 0.36%.
The diff coverage is 92.85%.

@@            Coverage Diff             @@
##           master      #24      +/-   ##
==========================================
- Coverage   91.89%   91.52%   -0.37%     
==========================================
  Files           4        4              
  Lines         111      118       +7     
  Branches       41       45       +4     
==========================================
+ Hits          102      108       +6     
- Misses          9       10       +1

Impacted Files	Coverage Δ
src/metadata/metadataVisitor.ts	93.75% <100%> (+0.89%)	⬆️
src/parameter/parameterVisitor.ts	95.83% <90.9%> (-4.17%)	⬇️
src/metadata/serializeType.ts	89.7% <0%> (ø)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3b1ece6...5dfcc90. Read the comment docs.

[leonardfactory]

@wtho I'm willing to merge and release this asap, however got some issues with type declarations from master. Just let me check them.