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
Closed
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
155 changes: 155 additions & 0 deletions text/0000-decorators.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,155 @@
- Start Date: 2018-02-21
- RFC PR: (leave this empty)
- React Issue: (leave this empty)

# Summary

This is a WIP RFC to discuss how we can develop the React ecosystem with the upcoming ECMA262 feature "Decorators".

[Decorators](https://github.com/tc39/proposal-decorators) is an ECMA262 proposal at stage 2 and yet to be standardised.

**Please note that decorators proposal have been through breaking changes. Please read the current version if you were unaware: [Link to Decorators Proposal](https://github.com/tc39/proposal-decorators)**

While it's not yet a standard, how they could be used in React ecosystem should be discussed for benefit of the React community and the standardisation process.

Just like React uses JSX at its core, it could also make use of another language future, decorators to enable better development experiences.

# Examples

## React Use: Component Flags

Instead of extending a separate class or nesting components for flagging a component, decorators can be used:
```javascript
// 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.

{/* ... */}
</AsyncMode>
)
}

// 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.

```

## React Use: Refs

A field decorator can be implemented to allow easier use of Refs:
```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.


myMethod = () => {
foo(this.myTextInput.value);
}

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.

)
}
}
```

## Community Use: HOCs

Many libraries like Redux and Relay provide additional Props to Components. They do this by wrapping the Component in another Component. Class decorators can be used to simplify consumption.

```javascript
// Before:
import { withRouter, connect } from './hocs';

class MyComponent extends Component { /* ... */ }

const MyComponentWithRouter = withRouter(MyComponent);

export default connect(/* ... */)(MyComponentWithRouter);

// After
import { withRouter, connect } from './hocs';

@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.)

```

## Community Use: Providing Mock Data

A decorator can be implemented for attaching mock data to a Component to help out-of-context rendering:

```javascript
type Props = {
showDetails: string, // From Redux
user: User, // From Relay
}

@MyLib.mock({
showDetails: false,
user: MyLib.fakeUser,
})
class MyComponent { /* ... */ }
```

Out-of-context rendering enabled by using such a decorator can allow design tools (like Facebook Origami and Adobe XD) or IDEs (like Atom and VSCode) to work on individual files.

# END OF DOCUMENT, THE REST WILL BE FILLED LATER

# Motivation

Why are we doing this? What use cases does it support? What is the expected
outcome?

Please focus on explaining the motivation so that if this RFC is not accepted,
the motivation could be used to develop alternative solutions. In other words,
enumerate the constraints you are trying to solve without coupling them too
closely to the solution you have in mind.

# Detailed design

This is the bulk of the RFC. Explain the design in enough detail for somebody
familiar with React to understand, and for somebody familiar with the
implementation to implement. This should get into specifics and corner-cases,
and include examples of how the feature is used. Any new terminology should be
defined here.

# Drawbacks

Why should we *not* do this? Please consider:

- implementation cost, both in term of code size and complexity
- whether the proposed feature can be implemented in user space
- the impact on teaching people React
- integration of this feature with other existing and planned features
- cost of migrating existing React applications (is it a breaking change?)

There are tradeoffs to choosing any path. Attempt to identify them here.

# Alternatives

What other designs have been considered? What is the impact of not doing this?

# Adoption strategy

If we implement this proposal, how will existing React developers adopt it? Is
this a breaking change? Can we write a codemod? Should we coordinate with
other projects or libraries?

# How we teach this

What names and terminology work best for these concepts and why? How is this
idea best presented? As a continuation of existing React patterns?

Would the acceptance of this proposal mean the React documentation must be
re-organized or altered? Does it change how React is taught to new developers
at any level?

How should this feature be taught to existing React developers?

# Unresolved questions

Optional, but suggested for first drafts. What parts of the design are still
TBD?