di.js icon indicating copy to clipboard operation
di.js copied to clipboard

Using angular/di.js in a babel runtime gives error

Open dmtrs opened this issue 9 years ago • 23 comments

My setup is babel v5.4.3 and mocha testing suite and I am trying to use di ^2.0.0-pre-14.

I have a require hook for my tests that looks like:

require("babel/register")({
    stage: 0
});

I have created a a file names src/rest.js that looks like this (request-promise):

import {Inject} from 'di';
import {default as Request} from 'request-promise';

@Inject(Request)
export class Rest {
    constructor(request) {
        this.request = request;
    }
}

following my src/rest.test.js looks like this:

import { Injector } from "di";
import { Rest } from "./rest";
import { should } from "chai";
should();

describe('Rest', function(){
    var injector = new Injector([]);
    describe('#constructor()', function(){
        it('should x', function(){ })
    })
})

when running the test I get the following error:

./node_modules/mocha/bin/mocha --require babelhook.js src/rest.test.js

/Users/dmtrs/code/sc.io/node_modules/di/dist/cjs/annotations.js:12
  this.tokens = tokens;
              ^
TypeError: Cannot set property 'tokens' of undefined
    at Inject (/Users/dmtrs/code/sc.io/node_modules/di/dist/cjs/annotations.js:12:15)
    at /Users/dmtrs/code/sc.io/src/rest.js:1:15
    at Object.<anonymous> (/Users/dmtrs/code/sc.io/src/rest.js:5:18)
    at Module._compile (module.js:456:26)
    at normalLoader (/Users/dmtrs/code/sc.io/node_modules/babel/node_modules/babel-core/lib/babel/api/register/node.js:150:5)
    at Object.require.extensions.(anonymous function) [as .js] (/Users/dmtrs/code/sc.io/node_modules/babel/node_modules/babel-core/lib/babel/api/register/node.js:163:7)
    at Module.load (module.js:356:32)
    at Function.Module._load (module.js:312:12)
    at Module.require (module.js:364:17)
    at require (module.js:380:17)
    at Object.<anonymous> (/Users/dmtrs/code/sc.io/src/rest.test.js:1:30)
    at Module._compile (module.js:456:26)
    at normalLoader (/Users/dmtrs/code/sc.io/node_modules/babel/node_modules/babel-core/lib/babel/api/register/node.js:150:5)
    at Object.require.extensions.(anonymous function) [as .js] (/Users/dmtrs/code/sc.io/node_modules/babel/node_modules/babel-core/lib/babel/api/register/node.js:163:7)
    at Module.load (module.js:356:32)
    at Function.Module._load (module.js:312:12)
    at Module.require (module.js:364:17)
    at require (module.js:380:17)
    at /Users/dmtrs/code/sc.io/node_modules/mocha/lib/mocha.js:192:27
    at Array.forEach (native)
    at Mocha.loadFiles (/Users/dmtrs/code/sc.io/node_modules/mocha/lib/mocha.js:189:14)
    at Mocha.run (/Users/dmtrs/code/sc.io/node_modules/mocha/lib/mocha.js:422:31)
    at Object.<anonymous> (/Users/dmtrs/code/sc.io/node_modules/mocha/bin/_mocha:398:16)
    at Module._compile (module.js:456:26)
    at Object.Module._extensions..js (module.js:474:10)
    at Module.load (module.js:356:32)
    at Function.Module._load (module.js:312:12)
    at Function.Module.runMain (module.js:497:10)
    at startup (node.js:119:16)
    at node.js:935:3

dmtrs avatar May 16 '15 21:05 dmtrs

The latest commit fixes this, but it's not published on npm yet, one of the Googlers need to do that. In the mean time, you can point to the github repo in package.json

caitp avatar May 16 '15 21:05 caitp

@vicb or @btford or someone might be willing to publish an update just for convenience

caitp avatar May 16 '15 21:05 caitp

@caitp I wil have a look and close the issue. Thanks for the moment

dmtrs avatar May 16 '15 22:05 dmtrs

Unfortunately the issue remains. What I did was to clone the repo with latest commit 9fd8c04d9d3182e5f19128af26ab84b66e56e633 used gulp to build the cjs folder as is and then moved everything under my project's node_module/di folder.

And the issue still remains.

dmtrs avatar May 17 '15 13:05 dmtrs

I don't think so, post the contents of your compiled annotations file

caitp avatar May 17 '15 14:05 caitp

I meant post it to like a gist or something =) Can you move that off the issue tracker please. Anyways, there may be more work needed to get it working with traceur. It may also need to be updated to match the current proposed decorator semantics.

caitp avatar May 17 '15 15:05 caitp

@caitp I have removed the code and added to a gist.

You mean babel need update to much current proposed decorator semantics?

dmtrs avatar May 17 '15 15:05 dmtrs

No, I mean di.js may need an update to match the current es7 proposal for decorators. Or, it may not. Angular isn't really using this library at the moment, so it's not clear what's needed until people file bugs

caitp avatar May 17 '15 15:05 caitp

Could I overcome the issue if I use the following syntax?

import {annotate, Inject} from 'di';
import {default as Request} from 'request-promise';

export class Rest {
    constructor(request) {
        this.request = request;
    }
}
annotate(Rest, new Inject(Request));

and by this I mean if the behavior would be the same from part of the di.js lib

dmtrs avatar May 17 '15 16:05 dmtrs

I don't have the rights, may be @vsavkin ?

vicb avatar May 17 '15 18:05 vicb

This is because babel support decorators not annotations. See #104 for a fix.

L8D avatar May 18 '15 20:05 L8D

I'll release a new version today.

vsavkin avatar May 19 '15 19:05 vsavkin

Hy, but the problem is same, and @L8D decorators fix is not the perfect solution (I think).

tamascsaba avatar May 29 '15 08:05 tamascsaba

Babel does not support annotations, it supports decorators. Annotations and decorators have the same syntax. The only way this library can seamlessly support both decorators and annotations is to stop using ES6 classes in their definitions (to allow the constructors to be called without 'new'), and then to add a check as to whether they are being called (as decorators) or initialized (as annotations) and then to return the appropriate values (the decorator function or the annotation instance).

Something along the lines of this:

function Inject(...tokens) {
  if (this instanceof Inject) { // being initialized as an annotation
    this.tokens = tokens;
    this.isPromise = false;
    this.isLazy = false;
  } else { // being called as a decorator
    return function(fn) {
      annotate(fn, new Inject(...tokens));
    };
  }
}

The only other options are:

  • Provide the annotate function as a decorator, which would be used as @annotate(new Inject(Foo, Bar))
  • Provide explicit decorator functions, which would be used as @inject(Foo, Bar)

L8D avatar May 29 '15 17:05 L8D

Thanks, the quick answer, i think it is not so hard implement it to di,js. What do you think @vsavkin which is the ideal solution?

tamascsaba avatar May 29 '15 18:05 tamascsaba

In the long run DI.js should only support decorators. Angular 2, for instance, will drop support for annotations and will support only decorators very soon.

However, to make the transition easier, we can support both decorators and annotations for a short period of time.

To do that we need to do the following:

  • Implement a way to convert an annotation into a decorator. Similar to how it is done in Angular 2: https://github.com/angular/angular/blob/master/modules/angular2/src/util/decorators.ts#L3
  • Update readAnnotations to read either annotations or decorators. See here: https://github.com/angular/angular/blob/master/modules/angular2/src/reflection/reflection_capabilities.ts#L88

If anyone submits a PR with the fix, I'll merge it.

vsavkin avatar Jun 09 '15 17:06 vsavkin

@vsavkin why not the solution I proposed?

L8D avatar Jun 09 '15 18:06 L8D

@L8D In general I prefer not to rely on this pattern. A metadata record and the act of attaching that metadata record to an entity are two different things to me. For instance, the metadata can/should be made immutable. But I don't feel super strongly about it.

But I am more concerned with how we store the metadata. The ES7 spec defines the metadata API that is supposed to be used for this purpose. Using our own custom metadata storage (the annotations property) seems like a smell to me.

vsavkin avatar Jul 01 '15 15:07 vsavkin

@vsavkin what about storing annotations in symbol properties?

L8D avatar Jul 06 '15 13:07 L8D

@L8D definitely better than what we do today. Why don't you want to use the metadata API? Because of the polyfill?

vsavkin avatar Jul 06 '15 15:07 vsavkin

Any progress?

nmabhinandan avatar Jul 28 '15 12:07 nmabhinandan

Any update on this?

bugthesystem avatar Mar 15 '16 15:03 bugthesystem