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

[WIP] ES Decorators and React #31

Closed
wants to merge 1 commit into from

Conversation

AlicanC
Copy link

@AlicanC AlicanC commented Mar 8, 2018

A WIP RFC to discuss how ES Decorators can benefit React ecosystem.

@AlicanC
Copy link
Author

AlicanC commented Mar 8, 2018

A recent tweet by @acdlite (read here) discusses a new pattern. That pattern and this RFC both covers a way to provide extra data to components so whoever has interest in this RFC should also read that.

@AlicanC AlicanC changed the title Decorators WIP RFC [WIP] ES Decorators and React Mar 8, 2018
@AlicanC
Copy link
Author

AlicanC commented Mar 8, 2018

Other ways of providing extra data are HOCs and Render Props

There are old-ES-Decorators-style HOCs ((Config) => (ComponentType) => ComponentType):

And also Relay HOCs ((Config, ComponentType) => ComponentType):

// After:
@React.pure
@React.async
export default class MyComponent extends React.Component {}
Copy link

@streamich streamich Mar 8, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This can be done by

export default React.pure(React.async(MyComponent));

Class decorator is a function that consumes a class and may return a class. If there is a HOC that consumes a React component and returns a stateful react component, then you can already use it as a class decorator.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Decorators:

@React.pure
@React.async
export default class MyComponent extends React.Component {
    // ...Maybe hundreds of lines...
}

Nested Calls:

class MyComponent extends React.Component {
    // ...Maybe hundreds of lines...
}

export default React.pure(React.async(MyComponent));

Nested Calls are harder to write and read, produce worse diffs and requires you to scroll through possibly hundreds of lines to be seen.

Whatever can be done with decorators can already be done in one way or another. I just believe that decorators are easier to use. Consider it sugar.

Copy link

@yordis yordis Mar 8, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@streamich everything can be done by functions calls at the end of the day

const NavigationStyled = withStyles(styles)(NavigationComponent);
const NavigationStyledConnected = connect(
  mapStateToProps,
  mapDispatchToProps
)(NavigationStyled);
const NavigationStyledConnectedWithRouter = withRouter(NavigationStyledConnected)

export { NavigationStyledConnectedWithRouter as Navigation }

vs

@withStyles(styles)
@connect(mapStateToProps, mapDispatchToProps)
@withRouter()
class Navigation extends React.Component { }
export Navigation

I would prefer to focus on how this would improve our code base and workflows. Everything could be a function call at the end of the day, but fundamentality having something like this would improve the coding.

P.S: despite the current decorator state and opinions about mutating things inside or not, decorators are not evil, engineers are evil.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@yordis

You might be interested in pipeline operator.

P.S. I like decorators and use them all the time, I just don't really understand what is the discussion about? If you like @withRouter as a decorator, you can already use it. Or is about bringing decorators into React core?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are using Stage 0 Decorators. TypeScript doesn't have Stage 2 Decorators.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I use whatever decorators TypeScript supports, but thanks for the info, will look into the differences.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@streamich as an Elixir developer I definitely prefer the pipe operator but that is not my point.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pipe operators are nice, but still, I would prefer to use decorators at class level. I think that pipe operators are more useful for minor operations such as helper function calls, but they would look horrible at class level.

```javascript
class MyComponent extends Component {
@React.ref
myTextInput;
Copy link

@streamich streamich Mar 8, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't it better to just assign the value?

myTextInput = React.ref();

Also, I believe that instance property decorators are broken in TypeScript, correct me if I'm wrong.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See the other comment for explanation.

TypeScript has support for old ES Decorators and it's under a flag. They jumped the gun and implemented a non-standard and fed it to people through Angular. TypeScript's mess doesn't and shouldn't affect ECMA262 or React ecosystems.


render() {
return (
<TextInput ref={this.refs.myTextInput} />

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need .refs here?

<TextInput ref={this.myTextInput} />

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

myTextInput is typed ?TextInput. this.refs contains hints for every property decorated with @React.ref. React uses those hints to automatically build a ref callback like (ref) => { instance[propertyName] = ref; }.

Naming could be different or this could be a stupid idea after all.


@withRouter
@connect(/* ... */)
export default class MyComponent extends Component { /* ... */ }
Copy link

@streamich streamich Mar 8, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Correct me if I'm wrong, but you can already do this, if withRouter() and connect() return stateful class components.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(Also correct me if I'm wrong, but) ES Decorators spec has changed and their current implementations are no longer valid decorators. Current decorators do not return a new class. (AFAIK, finalising the decoration with a new class is still possible though.)

@AlicanC
Copy link
Author

AlicanC commented Mar 8, 2018

I'd like to note that the main aim of the RFC is talk about not React's code but its ecosystem. I can imagine many conclusions for this RFC:

  • React starts providing decorator APIs.
  • Libraries start providing decorator APIs.
  • We write documentation to tell users how to use decorators. (Like there is for HOCs and Render Props)
  • All, some or none of the above.

While I follow many well-considered React people, I haven't seen anyone talking about decorators and how they can (or maybe can't) benefit React.

I just wanted to start a discussion and get people thinking with decorators. I will edit the RFC as we further the discussion.

I believe this will be an easier discussion after Stage 2 Decorators support for Babel 7 is complete: babel/proposals#11

Until then it will not be possible to provide POCs.

@vcarl
Copy link

vcarl commented Mar 8, 2018

As you note, Decorators are stage 2. I believe the main reason they haven't been discussed more broadly is because they're still viewed as unstable and likely to cause churn. The confusion between stage 0 and stage 2 decorators in the comments above seems to be evidence to that point, it seems very possible that they'll undergo further changes if and when they progress to stage 3.

And if it's not currently possible to provide POCs because they're not supported anywhere, then that again seems like evidence that this conversation should be happening sometime in the future.

@AlicanC
Copy link
Author

AlicanC commented Mar 8, 2018

it seems very possible that they'll undergo further changes

That's the important part. We can either wait until TC39 finalises the spec, or shape the spec by thinking on valid use cases. It would be great if we could discuss how we could benefit from decorators and provide TC39 with constructive inputs.

@yordis
Copy link

yordis commented Mar 8, 2018

As you note, Decorators are stage 2. I believe the main reason they haven't been discussed more broadly ...

@vcarl the reason why I would love React team to talk about this so the specification moves faster and we can benefit from this feature, but because we keep looking for workaround (or rather other way to do it) we do not get there 😞

From my code example, either pipe operator or decorators but the current way to do is so ugly and easy to error prune to be honest.

@AlicanC
Copy link
Author

AlicanC commented Mar 9, 2018

HOCs with Decorators

HOC (Note: This is NOT a valid Stage 2 Decorator)

const injectFoo = (value) => (Base) =>
  class Wrapper extends Component {
    render() {
      return <Base foo={value} />;
    }
  };
const injectBar = /* ... */;
const injectBaz = /* ... */;

class MyComponent extends Component {
  render() {
    const { foo, bar, baz } = this.props;
  }
}

export default injectFoo('foo')(injectBar('bar')(injectBaz('baz')(MyComponent)));

HOC w/ Stage 2 Decorators

const injectFoo = (value) => (classDescriptor) => ({
  ...classDescriptor,
  // Finisher is a HOC
  finisher: (Base) =>
    class Wrapper extends Component {
      render() {
        return <Base foo={value} />;
      }
    },
});
const injectBar = /* ... */;
const injectBaz = /* ... */;

@injectFoo('foo')
@injectBar('bar')
@injectBaz('baz')
export default class MyComponent extends Component {
  render() {
    const { foo, bar, baz } = this.props;
  }
}

Preventing Prop Name Collision with Property Decorators

Apollo has a HOC named "graphql" and it injects a prop named "data". To prevent collisions you have to pass an options object for each use:

import { graphql } from 'react-apollo';

class MyComponent extends Component {
  render() {
    const { foo, bar, baz } = this.props;
  }
}

export default compose(
  graphql(foo, { name: 'foo' }),
  graphql(bar, { name: 'bar' }),
  graphql(baz, { name: 'baz' }),
)(MyComponent);

This could be simplified with property decorators:

import { graphql } from 'react-apollo';

const graphqlProp = (query, options) => (elementDescriptor) => {
  const propName = elementDescriptor.key;
  
  return {
    ...elementDescriptor,
    propertyDescriptor: {
      ...elementDescriptor.propertyDescriptor,
      writable: false,
      get() {
        return this.props[propName];
      },
    },
    // Finisher is a wrapped HOC
    finisher: (Base) => graphql(query, {
      ...options,
      name: propName,
    })(Base),
  }
};

export default class MyComponent extends Component {
  @graphqlProp(foo) foo;
  @graphqlProp(bar) bar;
  @graphqlProp(baz) baz;

  render() {
    // Both work
    const { foo, bar, baz } = this.props;
    const { foo, bar, baz } = this;
  }
}

// Before:
export default class MyComponent extends React.PureComponent {
return (
<AsyncMode>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note that AsyncMode applies to the whole subtree. It's not specific to a component. You'd normally only use it once at the top. So I think it's not very relevant to this proposal.

@adrianhelvik
Copy link

adrianhelvik commented Nov 14, 2018

I use decorators a lot for providing access to some context, such as confirm dialogues and status messages. A nice bonus is that they can render differently in different contexts. Also, I prefer it to using render props. With render props I don't have access to the received state from this. Would love to have:

@withContext('foo', MyContext)
class Foo extends Component {
  render() {
    return (
      <div>
        {this.props.foo.bar}
      </div>
    )
  }
}

This would be just as cool however:

class Foo extends Component {
  static withContext = {
    foo: MyContext
  }

  render() {
    return (
      <div>
        {this.props.foo.bar}
      </div>
    )
  }
}

@gaearon
Copy link
Member

gaearon commented Feb 10, 2020

#68

@gaearon gaearon closed this Feb 10, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

8 participants