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

Decorator not applied on same react child components #16

Open
nd0ut opened this Issue Jan 26, 2016 · 6 comments

Comments

Projects
None yet
3 participants
@nd0ut

nd0ut commented Jan 26, 2016

var count = 0;

@connect(state => ({
    products: state.products
}))
export default class ParameterNode extends React.Component {
    constructor(props, context) {
      super(props, context);
      console.log('props', props);
    }

    render() {
        count = count + 1;

        if(count < 5) {
            return <ParameterNode />;
        }
        else {
            return <div></div>;
        }
    }

}

And the console output:

props Object {products: Object}
props Object {}
props Object {}
props Object {}
props Object {}

On the babel 5 it's ok.

@loganfsmyth

This comment has been minimized.

Show comment
Hide comment
@loganfsmyth

loganfsmyth Jan 26, 2016

Owner

Would you be able to trim down a reproducible version with a simple decorator? I don't know how connect works. If not I can give it a shot eventually.

Owner

loganfsmyth commented Jan 26, 2016

Would you be able to trim down a reproducible version with a simple decorator? I don't know how connect works. If not I can give it a shot eventually.

@nd0ut

This comment has been minimized.

Show comment
Hide comment
@nd0ut

nd0ut Jan 27, 2016

@loganfsmyth hmm, it seems that there is problem with react-redux connect decorator cause with simple decorator all works fine. Sorry for disturbing.

nd0ut commented Jan 27, 2016

@loganfsmyth hmm, it seems that there is problem with react-redux connect decorator cause with simple decorator all works fine. Sorry for disturbing.

@nd0ut nd0ut closed this Jan 28, 2016

@nd0ut nd0ut reopened this Jan 28, 2016

@nd0ut

This comment has been minimized.

Show comment
Hide comment
@nd0ut

nd0ut Jan 28, 2016

I was wrong.

function wrap(wrappedComponent) {
    class Wrapper extends React.Component {
        render() {
            return React.createElement(wrappedComponent, this.props);
        }
    }
    return Wrapper;
};

@wrap
export class Node extends React.Component {

    render() {
        const nodes = [
            <Node alone />,
            <Node alone />,
            <Node alone />
        ];

        if(this.props.alone) {
            return <div>Node</div>;
        }
        else {
            return <div>{nodes}</div>;
        }
    }
}

As result: one wrapper, many nodes.

Expected: each node wrapped by wrapper (babel-5 behavior)

nd0ut commented Jan 28, 2016

I was wrong.

function wrap(wrappedComponent) {
    class Wrapper extends React.Component {
        render() {
            return React.createElement(wrappedComponent, this.props);
        }
    }
    return Wrapper;
};

@wrap
export class Node extends React.Component {

    render() {
        const nodes = [
            <Node alone />,
            <Node alone />,
            <Node alone />
        ];

        if(this.props.alone) {
            return <div>Node</div>;
        }
        else {
            return <div>{nodes}</div>;
        }
    }
}

As result: one wrapper, many nodes.

Expected: each node wrapped by wrapper (babel-5 behavior)

@loganfsmyth

This comment has been minimized.

Show comment
Hide comment
@loganfsmyth

loganfsmyth Jan 28, 2016

Owner

Perfect, thanks for the great example. I've got a busy week, but I'll try to get to this when I can.

Owner

loganfsmyth commented Jan 28, 2016

Perfect, thanks for the great example. I've got a busy week, but I'll try to get to this when I can.

@loganfsmyth

This comment has been minimized.

Show comment
Hide comment
@loganfsmyth

loganfsmyth Jan 31, 2016

Owner

I'm a little on the fence on this, now that I look into it more. It seems like an edge-case where Babel 5's behavior differers from my reading of the spec. The https://github.com/wycats/javascript-decorators#desugaring-es6 ES6 class desugaring example from the decorator repo is the logic that I follow in this implementation. My reading of that would imply that usage of Note within the Note class body, would still refer to the original class constructor, not the one returned from the decorator function.

At the moment, I'm tempted to lean toward adding this as a third entry in the "Best Effort" list in the README.

Owner

loganfsmyth commented Jan 31, 2016

I'm a little on the fence on this, now that I look into it more. It seems like an edge-case where Babel 5's behavior differers from my reading of the spec. The https://github.com/wycats/javascript-decorators#desugaring-es6 ES6 class desugaring example from the decorator repo is the logic that I follow in this implementation. My reading of that would imply that usage of Note within the Note class body, would still refer to the original class constructor, not the one returned from the decorator function.

At the moment, I'm tempted to lean toward adding this as a third entry in the "Best Effort" list in the README.

@amino-studio

This comment has been minimized.

Show comment
Hide comment
@amino-studio

amino-studio Jan 31, 2017

Any updates?

amino-studio commented Jan 31, 2017

Any updates?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment