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

metadata is undefined (Babel) #9

Closed
liangzr opened this issue Jul 20, 2017 · 10 comments
Closed

metadata is undefined (Babel) #9

liangzr opened this issue Jul 20, 2017 · 10 comments
Labels

Comments

@liangzr
Copy link

liangzr commented Jul 20, 2017

I create a simple advice just like Get Started, but advices can't find metadata.

  • simple react demo project
  • kaop-ts v1.2.0

App.js

import React, {Component} from 'react';
import logo from './logo.svg';
import './App.css';
import {afterMethod} from 'kaop-ts'
import {MyAdvices} from './MyAdvices'

class App extends Component {

  constructor(props) {
    super(props);
    this.handleClick = this.handleClick.bind(this);
  }

  @afterMethod(MyAdvices.doubleResult)
  handleClick() {
    alert('You Click It!');
  }

  render() {
    return (
      <div className="App">
        <div className="App-header" onClick={this.handleClick}>
          <img src={logo} className="App-logo" alt="logo"/>
          <h2>Welcome to React</h2>
        </div>
        <p className="App-intro">
          To get started, edit
          <code>src/App.js</code>
          and save to reload.
        </p>
      </div>
    );
  }
}

export default App;

MyAdvices.js

import { AdvicePool, adviceMetadata } from 'kaop-ts'

export class MyAdvices extends AdvicePool {
  static doubleResult (@adviceMetadata metadata) {
    console.log(metadata);     // always output undefined here
  }
}
@k1r0s
Copy link
Owner

k1r0s commented Jul 20, 2017

Hi there! Thanks for oppening the issue.

I was not able to reproduce with a custom test (check below)

  "devDependencies": {
    "@types/react": "^0.14.36",
    "@types/react-dom": "^0.14.17",
    "awesome-typescript-loader": "^2.2.4",
    "typescript": "2.0.3",
    "webpack": "^1.9.6"
  },
  "dependencies": {
    "kaop-ts": "^1.2.0",
    "react": "^15.3.1",
    "react-dom": "^15.3.1"
  }

I Think the problem is that you're overriding method handleClick in your constructor by assigning the result of .bind function:

  constructor(props) {
    super(props);
    this.handleClick = this.handleClick.bind(this); // avoid
  }

You should do this instead, in order to keep method behavior:

 <div className="App-header" onClick={this.handleClick.bind(this)}>

@liangzr
Copy link
Author

liangzr commented Jul 20, 2017

@k1r0s Thanks for reply
I modify handleClick method jusk like you did,but it's useless. I use babel for ES7 decorator in my code without typescript.

MyAdvices.js

import { AdvicePool, adviceMetadata } from 'kaop-ts'

export class MyAdvices extends AdvicePool {
  static doubleResult (@adviceMetadata metadata) {
    console.log(metadata);     // always output undefined here
  }
}

After transform it with babel-cli, I can't find anything about adviceMetadata and decorator, maybe they disappeared when transformation?

After transform, partial code

var MyAdvices = exports.MyAdvices = function (_AdvicePool) {
  _inherits(MyAdvices, _AdvicePool);

  function MyAdvices() {
    _classCallCheck(this, MyAdvices);

    return _possibleConstructorReturn(this, (MyAdvices.__proto__ || Object.getPrototypeOf(MyAdvices)).apply(this, arguments));
  }

  _createClass(MyAdvices, null, [{
    key: 'doubleResult',
    value: function doubleResult(metadata) {
      console.log(metadata);
    }
  }]);

  return MyAdvices;
}(_kaopTs.AdvicePool);

@k1r0s
Copy link
Owner

k1r0s commented Jul 20, 2017

I have to check this in deep.

This lib isn't coded against babel since they dropped support for decorators some time ago ..

Please share your package.json in order to test it. Thanks!

@k1r0s k1r0s added the feature label Jul 20, 2017
@k1r0s k1r0s changed the title metadata is undefined metadata is undefined (Babel) Jul 20, 2017
@liangzr
Copy link
Author

liangzr commented Jul 20, 2017

At first I added kaop-ts for my project and configrue babel with webpack, but it doesn't work with the same bug.So I create a simple demo by create-react-app, and use babel-cli to transform App.js and MyAdvices.js manually.

package.json

{
  "name": "aspect-demo",
  "version": "0.1.0",
  "private": true,
  "dependencies": {
    "kaop-ts": "^1.2.0",
    "lodash": "^4.17.4",
    "react": "^15.6.1",
    "react-dom": "^15.6.1"
  },
  "devDependencies": {
    "babel-plugin-transform-decorators-legacy": "^1.3.4",
    "babel-preset-latest": "^6.24.1",
    "babel-preset-react": "^6.24.1",
    "babel-preset-stage-2": "^6.24.1",
    "chai": "^4.1.0",
    "mocha": "^3.4.2",
    "react-scripts": "1.0.10",
    "sinon": "^2.3.8"
  },
  "scripts": {
    "start": "react-scripts start",
    "build": "react-scripts build",
    "test": "react-scripts test --env=jsdom",
    "eject": "react-scripts eject"
  }
}

.babelrc

{
    "presets": [
      "latest",
      "react",
      "stage-2"
    ],
    "plugins": ["transform-decorators-legacy"]
  }

@k1r0s
Copy link
Owner

k1r0s commented Jul 23, 2017

Hi again @liangzr ,

Main problem with babel is about param decorators.

In order to retrieve metadata you need @adviceMetadata decorator which isn't working on babel as I have tested.

Typescript implemented this some time ago. While babel firstly drop support because this feature wasn't fully included in decorators proposal.. (param decorators wasn't included initially). More info: babel/babel#1301

And now are working to implement that feature: babel/proposals#13

There are so call plugins to enable param decorators in babel but none of them worked for me in babel.

I'll keep an eye on this, but I recommend to use this library with typescript.

There is a JS vanilla version https://github.com/k1r0s/kaop which provides AOP tools in browsers world (ES5)

That's all :(

Hope @babel keep working on this

@liangzr
Copy link
Author

liangzr commented Jul 24, 2017

Thanks a lot @k1r0s ,

It is difficult to replace the whole project into typescript, but I wirte advices files use typescript (that I learned just last weekend :P) according to your suggestion, use babel for other .js files, it works fine now!

I also will keep an eye on this, thanks for you libarary!

@k1r0s
Copy link
Owner

k1r0s commented Jul 24, 2017

Nice. Hope Babel keep pushing decorators feature

Any doubts give me a touch :)

I think we can close this issue. Thanks a lot

@k1r0s k1r0s closed this as completed Jul 24, 2017
@k1r0s
Copy link
Owner

k1r0s commented Jul 30, 2017

Hey @liangzr, checkout last release :)

@liangzr
Copy link
Author

liangzr commented Aug 10, 2017

@k1r0s gread job!

Although I prefer to use typescript now, but there are a lot of people use original javasciprt,which is very useful for them.

@k1r0s
Copy link
Owner

k1r0s commented Aug 11, 2017

Nice if you take that decision! Typescript is wonderful

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

No branches or pull requests

2 participants