gaearon/babel-plugin-react-transform

Support Functional Components

jamiebuilds opened this issue · 64 comments

Moving from #34.

As of react@0.14, plain functions can be used as components. In the future this will bring performance optimizations in React.

function Component(props) {
  return <div/>;
}

Many people are already using these, there's even a Babel plugin (that I wrote) to transform pure component classes to pure component functions.

It would be great if babel-plugin-react-transform supported these. However, there is a pretty big concern.

Since these are just plain functions, with JSX in them there is nothing that designates that it is a react component. i.e. in order to tell that a class is a react component, we can test that it extends React.Component, or that it has a matching interface.

We can attempt to match them by treating "any function that has JSX in it will be treated as a component" i.e.:

function Component() {
  return <div/>;
}

However, this would also match the inner function in here:

class Component extends React.Component {
  render() {
    const children = this.props.items.map(function(props) {
      return <li>{props.name}</li>;
    });
    return <ul>{children}</ul>;
  }
}

So we can tighten things and say "only top level components", but then we run into wanting to support factories:

function componentFactory() {
  return function Component(props) {
    return <div/>;
  };
}

I could keep going for a while, you can put functions all over the place in JavaScript and no matter what we define as the rule here there will be exceptions. It's a matter of deciding what the heuristics should be that balances:

  1. Rarity - How often people will actually run into exceptions in real world code
  2. Obviousness - How easy it is to understand the reasoning behind an exception

It might be possible to get around the obviousness problem by throwing compile errors when running into ambiguous scenarios.

If you only do the top level ones automatically and would require a comment above or inside other functions? // @hmr: true or something

No, comments are a gross for this.

We get a ton of issues on Babel because of the Angular community using /* @ngInject */ comments to modify their code. Because comments are insignificant syntax they run into all sorts of problems.

I would try everything possible before using comments.

I'd choose a convention a la "let/const function bindings at top level with capital letter names and JSX immediately inside."

In the first release I'd avoid trying to find factories altogether. I don't think that's a big problem.

Avoiding false positives is more important than finding every possible case. You can look at eslint React plugin as an inspiration for the heuristics.

@thejameskyle I think what @gaearon suggests makes sense and as of next iteration we can support a "manual hint" case when heuristic didn't work. What are other options yo see besides comments? Can generators be used?

An additional heuristic can be "a function with propTypes assigned to it later". Though React is gradually phasing out propTypes in favor of flow, it seems like a nice heuristic for most code bases.

Can we limit the heuristic to any function that returns JSX and has props as its first argument?

Can we limit the heuristic to any function that returns JSX and has props as its first argument?

Unfortunately, that would not cover the destructuring use case:

(from https://facebook.github.io/react/blog/2015/10/07/react-v0.14.html#stateless-functional-components)

// Or with destructuring and an implicit return, simply:
var Aquarium = ({species}) => (
  <Tank>
    {getFish(species)}
  </Tank>
);

The other issue that I didn't point out here is that many people do not use JSX in favor of something like hyperscript.

@SpainTrain good point. I am even using destructuring in some of my stateless components. 😄

@thejameskyle Shouldn't the first pass be focused only on the most common and recommended way (JSX/ES6) of writing stateless components?

So far proposed heuristics are:

  • let/const function bindings at top level with capital letter names and JSX immediately inside
  • a function with propTypes assigned to it later

This doesn't cover:

  • non-JSX users that don't use propTypes
  • factories and some HOCs

And it's sounds like it's okay for now. Subjectively this covers majority of the cases.

We haven't yet discussed manual hinting, since I feel like it's a nice option.

I think that going with something as basic as "let/const function bindings at top level with capital letter names and JSX immediately inside." is probably a great idea.

Right now, people are just saying "Please support functional components!"

Once you can say "They are supported as long as... xyz" then you will probably start getting people with issues letting you know when there is a scenario that "xyz" just doesn't work.

insin commented

How about TitleCase functions with a return statement which isn't guarded by a !(this instanceof ...) check, and React either imported or in function scope?

False positives would be constructors which handle creating more specialised instances which also happen to do something with React/JSX.

Could be wrong, but a functional component could be defined as:

  • name starts with a capital
  • has JSX directly inside
  • does not contain a render method
    Would that work for factories?

Regarding manual hinting, would something like this work:

"HMR"; var Component = function() {
    return <div />;
}
// or:
var Component = function() { 
    "HMR"; 
    return <div />;
}

It's not a comment, and this pattern is used for strict mode ("use strict";), too.

@insin

with a return statement which isn't guarded by a !(this instanceof ...) check

Can you elaborate a little, why return statement that isn't guarded with mentioned check is a good indicator of a component?

How about the following for the heuristics?

  1. The name is PascalCase
  2. Returns JSX or null
  3. Accepts only one argument (even with destructuring it's just one argument)
  4. [Bonus] Has contextTypes, propTypes, defaultProps
  1. 👍 Convention usually has factory functions & decorators as camelCase.
  2. 👍 The presence of JSX seems like the best indicator to me, based on documentation, boilerplates, etc.
  3. I thought that pure functions received the signature (props, context)? So maybe not a reliable indicator.
  4. Even though it negates the savings of a "pure" component, opting in via propTypes would encourage a better development experience via both HMR & prop validation. Even with the slow movement towards Flow, it doesn't seem to have permeated enough to not use propTypes as an indicator.

My rationale is:

  • Pure components are the clearest way of defining a UI regarding best-practices. Minimizing hints (e.g. having to thoughtfully placing "HMR"; & even propTypes to improve the DX) is the ideal.

(Subscribed!)

@ericclemmons agree with points, except propTypes. I think it should be optional and used as "manual hinting" when nothing else works. One of the great benefits of Functional Components is their ergonomics, simplicity to define and use. Having a need to think about propTypes all the time reduces that feel of "lightweightness".

One could poke holes through all of the heuristics listed in this thread. I'm guessing everyone here is basing it off of how they choose to personally write these components which is bound to cause issues. I'm convinced that this is a bad idea to support.

I'm thinking it'd be a better idea to tell people not to use functional components and just run their code through: https://github.com/thejameskyle/babel-plugin-react-pure-components which will optimize it to functional components for them. I'll need to put more work into that though

@thejameskyle Why is that bad idea to support?

Not using functional components is a bigger drawback imo. It doesn't teach you to use functions and basically removes benefits of having component as a function (e.g. functional composition, less-friction HOCs).

Having a heuristic would help with that. Yes a lot of people would not benefit from it (non JSX users, HOCs and component factories), but this is unsolvable unless we can hot-swap any function at runtime that was changed, which goes far beyond this project and can be a plugin for Babel (convention based like "folder /bla contains only pure functions").

So the statement remains, let's cover it for cases we can, since it's sounds like it's a majority of them. Can be a configuration param "enable partial functional component support". Feels like so many people would benefit from it.

You don't understand, anything that is supported will affect cases it should not, in addition to missing other cases. There are no heuristics here that won't cause issues.

Debugging compiled code is not easy for most people, and even with documentation explaining edge cases, people get tripped up. Whether it be because they don't read docs or because a coworker did something without their knowledge.

A compiler, and by proxy compiler plugins, should not have edge cases that result in broken code, it's very bad.

@thejameskyle I see. What are the potential issues for "function that returns JSX"?

Not sure I understand your point about compiled code, while I agree with conclusions that it's not easy to debug, compiler should have no broken code, etc. But what compiled code we are talking about? Transforming non-functional component to a functional with mentioned Babel plugin can result in this, since it transforms components to functional ones. Did you mean that?

So in general I got it. Since it's "generic react component transform". But practically speaking what a lot of people would benefit from is the following:

  • hot reloading for most functional components
  • other debugging tools for functional components (re-render highlight, etc)

If we say that we can't support generic functional components transformations in this plugin it's fine. But I suggest to think how those debugging tools can still be supported with few sacrifices.

Worst case should be "something doesn't hot reloads, okay, I will just refresh the whole page or read docs how to manually hint it".

The main idea of this plugin was developer's experience and hot-reloading, wasn't it? So maybe we need a different, less ambitious Babel plugin for that? babel-plugin-react-debug-transform that allows to cover those cases?

@thejameskyle Would it be best for someone like myself to create the inverse of babel-plugin-react-pure-components (Pure functions in, opinionated hinting, classes out) so HMR is supported with pure functions?

That way compiler & build-step optimization can happen in production & people can write cleaner code w/o missing out on the DX?

Would it be best for someone like myself to create the inverse of babel-plugin-react-pure-components (Pure functions in, opinionated hinting, classes out) so HMR is supported with pure functions?

You'd run into even more problems if you tried to do that.

My recommendation is to never use functional components and simply compile them using that plugin.

@thejameskyle Ok, so is this the synopsis then?

  • HMR with functional components will not be supported since there is no single reliable way to identify them and instrument only them.
  • Functional components still offer a performance benefit, so it's still recommended to write React.createClass and leverage babel-plugin-react-pure-components in production builds.

Are we good to close this then? It looks like your original points don't have an agreeable solution:

#57 (comment)

babel-plugin-react-pure-components does not support React.createClass only es6 classes

I'm going to leave this open because I'm assuming @gaearon will disagree with me 🚎

I'm going to leave this open because I'm assuming @gaearon will disagree with me

Yeah I will. If we don't support at least some valid subset, we're effectively making this plugin useless because using functional components everywhere possible is official recommendation from React team (and I agree):

https://facebook.github.io/react/blog/2015/10/07/react-v0.14.html#stateless-functional-components

In idiomatic React code, most of the components you write will be stateless, simply composing other components. We’re introducing a new, simpler syntax for these components [...] This pattern is designed to encourage the creation of these simple components that should comprise large portions of your apps.

I'd rather support a subset and throw on cases where we aren't 100% sure what's going on, than not support them at all. I propose to base the subset on conventions based in the official React documentation—indeed it won't satisfy everyone but I don't care as long as there's a common case we support.

I need to add that I don't consider this to be a long term solution anyway. This project is experimental. @sebmarkbage also thinks it's fragile, and I completely agree.

The goal of this project is to show what's possible, provide a temporary solution demoing this great DX, and then find ways to get it inside React in a less fragile way. Another goal of this project was to do this independently of the bundler, so we can widen this experimentation to Browserify, React Native, and other platforms that don't use Webpack.

I think we did well on both of these goals. React should get experimental support for error boundaries (facebook/react#5602) in the next release, which removes the need for react-transform-catch-errors. There is also work on adding an official DevTools API to React (facebook/react#5306) which should satisfy scenarios accomplished by other transforms such as react-transform-render-visualizer.

react-transform-hmr is the odd one because it’s the only one really hard to do without the source transform. There might be ways to mitigate this on the React side, but I don't see any short-term solutions. If 0.15 ships with error boundaries and DevTools API I'll be happy to scope back this project to supporting just hot reloading (maybe rebrand it as React Hot Loader 2?) in a bundler-agnostic way. However I don't want to lose momentum until we get there, and I don't want to lose momentum afterwards while we try to figure out a way to make this work on React side without hacks.

Not supporting functional components = losing momentum. It will be bad for the mission of this project. Supporting a subset of them, even as opt-in, would be good. Because this project also exists to test the boundaries of what we can do, and the hard problems we're going to hit. It can afford to be a little bit reckless—if you want more guarantees just keep using React Hot Loader. And hopefully we can take what we learn here, and the issues that might arise, and apply that knowledge to make hot reloading first class experience in the React ecosystem.

@gaearon Shall we leave it to you & @thejameskyle to settle on the markers for enabling this?

There is ample discussion & recommendations in the thread, and we're willing to help out once the course has been chosen.

(I think we all agree on the brittle, or "reckless", nature of this, but it's true that HMR has been a powerful motivator this year for devs)

Exactly, HMR with React is a biggest WOW effect for now. This is what people buy (at least my observation) and this is what this project enables.

FWIW, this is how react-docgen detects stateless components: https://github.com/reactjs/react-docgen/blob/master/src/utils/isStatelessComponent.js

(I didn't read all the comments in this thread)

I think I have an idea how to implement hot reloading in React-agnostic way with a different transform that would be less fragile. Stay tuned ;-)

@gaearon That'd be fantastic! I'm experimenting on a transform/plugin that lets you use a different renderer per-build (for shipping React/Mithril/virtual-dom/etc.) via the same functional components:

https://github.com/ericclemmons/babel-plugin-jsx-demux

<Subscribed>

Hi,

I have discovered this issue after 2 days debugging while my hot reloading had stopped working. Unfortunately it wasn't obvious that it came from stateless components, as I had upgraded my project from Flux to redux, and changed to stateless components as a result all in the same commit.

I have read through the issue and I now understand why it is not yet supported due to the difficulty in knowing which functions are React stateless components.

I was wondering if there had been any advances made? What were the current state of things?

Thanks!

Going to submit a PR based on my work in ericclemmons/babel-plugin-react-pure-to-class#1 very soon.

It works great, can be entirely opt-in, and certainly cleans up the code.

Exciting!!!

If someone is looking for a more reliable method of doing hot module reloading I think http://chromakode.github.io/isolated-core/ is a really good solution.

Going to submit a PR based on my work in ericclemmons/babel-plugin-react-pure-to-class#1 very soon.

Can't wait!

If someone is looking for a more reliable method of doing hot module reloading I think http://chromakode.github.io/isolated-core/ is a really good solution.

It doesn’t support hot reloading React component local state or preserving the DOM which is the only reason you’d want to use this plugin anyway. If you don’t care about this, it is sufficient to write something like

if (module.hot) {
  module.hot.accept('./App', NextApp =>
    ReactDOM.render(<NextApp />, document.getElementById('root'))
  )
}

once in your index.js without any transforms.

Rejoice!

#85

Wohoo! Awesome!

For anyone still looking for this, if you use something like Redux for your state, I would suggest just ditching React Transform like shown here: reduxjs/redux#1455.

You’d still rely on HMR (don’t confuse it with proxying! HMR is just transport for scripts and module updates) but you would not need proxies, and you would rely on React to re-mount your components. This would work for any kinds of components, including functional ones.

Give it a try: reduxjs/redux#1455

👍 @gaearon, works like a charm and recommend it to others for hotloading functional components as well as redux !

amcsi commented

I don't understand. Has this answer been in front of us all along? And wasn't hmre needed for hot module replacement?

And if I get it straight, I just remove the react-hmre babel preset and then HMR will still work, functional components will also work, but in exchange local state won't work?

I wish I knew exactly how HMR worked internally.

MoOx commented

HMR is just something that allow you to write this kind of code

if (module.hot) {
  // do something when a file has been hot loaded
}

babel-plugin-react-transform just insert some code using this module.hot test to force rerender your component when they have been updated.

Without this plugin, you can still use hot loading (as explained before) to rerender your entire app. This will be "ok" if you have a global store like redux (since you are not supposed with this approach to rely too much on local state).

OK maybe we’ve been doing it from the wrong end all along:
https://twitter.com/dan_abramov/status/706681477157752835

@gaearon didn't quite follow your last twitter comment. Could you explain how it's relevant?

Edit: Found this medium post. Awesome read and all is clear now. :) https://medium.com/@dan_abramov/hot-reloading-in-react-1140438583bf#.fbkvfqotn

As mentioned in https://github.com/reactjs/redux/pull/1455/files#r56386442, I would love to see a real world example of the "basic" (non-proxied) hot reloading with functional components in a project with Redux and react-router. I can get it to work for a single component but can't work out how to get the updates to bubble up to a single parent component properly and am generally a bit confused :) but would like to work with stateless/functional components and some kind of HMR if possible

@tomduncalf don't have any open source project to show you at the moment, but you can take a look at this

// src/index.js
import 'babel-polyfill'
import 'whatwg-fetch'
import React from 'react'
import ReactDOM from 'react-dom'

const mountEl = document.getElementById('root')

let render = () => {
  const Root = require('./Root').default
  ReactDOM.render(<Root />, mountEl)
}

if (module.hot) {
  const renderApp = render

  const renderError = (error) => {
    const RedBox = require('redbox-react')
    ReactDOM.render(
      <RedBox error={ error } />, mountEl
    )
  }

  render = () => {
    try {
      renderApp()
    } catch (error) {
      renderError(error)
    }
  }

  module.hot.accept('./Root', () => {
    setTimeout(render)
  })
}

render()
// src/Root.js
import React from 'react'
import { Provider } from 'react-redux'
import { Router } from 'react-router'
import { hashHistory } from 'react-router'
import { syncHistoryWithStore } from 'react-router-redux'
import pure from 'recompose/pure'

import createStore from './store'
import routes from './routes'
import './styles/styles.scss'

const store = createStore()
const history = syncHistoryWithStore(hashHistory, store)

const Root = () =>
  <Provider store={ store }>
    <Router history={ history } routes={ routes } />
  </Provider>

export default pure(Root)
// src/routes.js
import React from 'react'
import { IndexRoute, Route } from 'react-router'
import { App, Home, NotFound } from './components/app'

export default (
  <Route path="/" component={ App }>
    { /* Main route */ }
    <IndexRoute component={ Home } />

    { /* Routes */ }

    { /* Catch all route */ }
    <Route path="*" component={ NotFound } status={ 404 } />
  </Route>
)

Thanks imran! I thought that was exactly what I did but there must be some subtle differences so I will try again :)

On 16 Mar 2016, at 18:36, Imran Ismail notifications@github.com wrote:

@tomduncalf don't have any open source project to show you at the moment, but you can take a look at this

// src/index.js
import 'babel-polyfill'
import 'whatwg-fetch'
import React from 'react'
import ReactDOM from 'react-dom'

const mountEl = document.getElementById('root')

let render = () => {
const Root = require('./Root').default
ReactDOM.render(, mountEl)
}

if (module.hot) {
const renderApp = render

const renderError = (error) => {
const RedBox = require('redbox-react')
ReactDOM.render(
<RedBox error={ error } />, mountEl
)
}

render = () => {
try {
renderApp()
} catch (error) {
renderError(error)
}
}

module.hot.accept('./Root', () => {
setTimeout(render)
})
}

render()
// src/Root.js
import React from 'react'
import { Provider } from 'react-redux'
import { Router } from 'react-router'
import { hashHistory } from 'react-router'
import { syncHistoryWithStore } from 'react-router-redux'
import pure from 'recompose/pure'

import createStore from './store'
import routes from './routes'
import './styles/styles.scss'

const store = createStore()
const history = syncHistoryWithStore(hashHistory, store)

const Root = () =>
<Provider store={ store }>
<Router history={ history } routes={ routes } />

export default pure(Root)
// src/routes.js
import React from 'react'
import { IndexRoute, Route } from 'react-router'
import { App, Home, NotFound } from './components/app'

export default (
<Route path="/" component={ App }>
{ /* Main route */ }
<IndexRoute component={ Home } />

{ /* Routes */ }

{ /* Catch all route */ }
<Route path="*" component={ NotFound } status={ 404 } />
) — You are receiving this because you were mentioned. Reply to this email directly or view it on GitHub

Sorry if I might be polluting this discussion with my lacking knowledge of how HMR is supposed to work, but could anyone explain to me why my attempt at a wrapper failed to solve this problem? https://gist.github.com/firstdoit/1fe09563cfe4dbb04246

The intended use was to simply wrap the export of any functional component and giving HMR a full React 'class' to play with.

Any updates on this? It would be awesome to use functional components without ditching the benefits of this plugin.

MoOx commented

@webyak This will likely never happen. See #57 (comment)

Not entirely “will never happen”. As I said in #57 (comment), supporting functional components will require a different approach but this doesn’t mean the benefits of this plugin are going away. You can track gaearon/react-transform-boilerplate#125 as something might happen there eventually, or read https://medium.com/@dan_abramov/hot-reloading-in-react-1140438583bf for more context.

React Hot Loader 3 supports functional components without destroying the state.
It supersedes both React Hot Loader and React Transform, and is built on lessons learned from both.
Check it out: gaearon/react-hot-boilerplate#61

Thanks @gaearon, can't wait to try it out!

amcsi commented

So let me get it straight: react hot loader was deprecated in favor of react transform, which in turn is deprecated back in favor of react hot loader again?

@amcsi React hot loader 3.0 is a combination of both React hot loader 1.0 and react transform. And it works with function component

amcsi commented

I don't quite get it, but all right.
Also, will this along with syntax error catching work just with webpack-dev-server and not having to do things manually with webpack-dev-middleware?

So let me get it straight: react hot loader was deprecated in favor of react transform, which in turn is deprecated back in favor of react hot loader again?

Yes. Both approaches were bad in different ways. I’m trying a third, mixed, approach, and it makes sense to brand it as React Hot Loader, as it doesn’t allow arbitrary transforms anymore.

Also, will this along with syntax error catching work just with webpack-dev-server and not having to do things manually with webpack-dev-middleware?

It is absolutely orthogonal: you can use either webpack-dev-server and webpack-hot-middleware with either of these three projects 😄 . That said the syntax error overlay is a feature of webpack-hot-middleware. If you’d like to see it in webpack-dev-server, feel free to file an issue there—none of my projects have any say in this at all, I only work on hot reloading React code itself, not the transport or middleware.