Radium: Radium wrapping not compatible with native classes?

[epsitec]

I've been successfully using Radium with the standard es2015 Babel preset.

Since our generated code runs on Node 5, I wanted to try out babel-preset-node5. With this preset, Babel is no longer translating class to ES5 constructs, since Node 5 supports classes out of the box.
However, when wrapping our component classes with Radium, I get this error:

TypeError: Class constructors cannot be invoked without 'new'

Here is a code snipped which reproduces the error:

import React from 'react';
import radium from 'radium';

class Foo extends React.Component {
  render () {
    return <div>Hello</div>;
  }
}

const WrappedFoo = radium (Foo);

new WrappedFoo (); // TypeError: Class constructors cannot be invoked without 'new'

My .babelrc:

{
  "presets": ["stage-0", "node5", "react"]
}

There seems to be some logic in the Radium wrapper that is not compatible with
_real_ ES2015 classes.

[Skywalker13]

In order to prevent the full es2015 preset, my workaround :-( :

  "plugins": [
        ...
        ["transform-class-properties"],
        ["transform-es2015-classes"],
        ...
  ]

[ianobermiller]

Yeah, Radium currently does not support native classes. I assume the fix would be similar to this: https://github.com/gaearon/react-proxy/pull/42

[gorillatron]

Having the same issue

[asvetliakov]

Can't you just provide separate es6 build?
i.e. import Radium from 'radium/es6'
The problem that super(...arguments) in ehnancer is being transformed by babel to:
var _this2 = _possibleConstructorReturn(this, _ComposedComponent.apply(this, arguments));

[eXaminator]

Are there any news on this? We just started creating an App on electron and are also using native classes and would like to use radium. I might get away with the above workaround for a while, but I really hope this will be fixed in the near future...

[ianobermiller]

The problem that super(...arguments) in ehnancer is being transformed by babel to:
var _this2 = _possibleConstructorReturn(this, _ComposedComponent.apply(this, arguments));

@kittens @thejameskyle Any idea if there is a workaround for this problem?

[jamiebuilds]

What's the problem? I don't follow

[asvetliakov]

Radium uses class inheritance. When trying to extend native ES6 class the _ComposedComponent will be native es6 class and both these lines will fail (which are transpiled result of extends/super())

      _classCallCheck(this, RadiumEnhancer);

      var _this2 = _possibleConstructorReturn(this, _ComposedComponent.apply(this, arguments));

[jamiebuilds]

Yeah, there isn't a way to make these interoperable. It's not possible to give the super class a context while instantiating it like this

[ianobermiller]

Thanks @thejameskyle, that helps. Alright, we have two options: bundle them together or publish a new NPM module with the untransformed code (somewhat risky, since you'd have to use all the same transforms we do). Another option would be for you to continue transpiling classes even where they are natively supported. Is that so bad?

[asvetliakov]

@ianobermiller
Please don't push fully untransformed code. Transform it to full es6 please. I'm using typescript for example and it can't transpile object rest spread for now.

[ianobermiller]

@asvetliakov ah yes that makes sense. So we could either have 'radium/es6' or 'radium-es6'.

[eXaminator]

+1 on transforming to ES2015 (or better yet: ES2016, though it doesn't make a big difference).

But maybe think about using npm namespaces instead of a complete new package name: http://blog.npmjs.org/post/116936804365/solving-npms-hard-problem-naming-packages

Maybe something like @radium/es6?

[begincalendar]

@eXaminator I think using NPM scopes for this purpose would be abusing NPM (in a sense), if we were to create a "radium" NPM username.
E.g. Imagine if every package wanted to achieve the same feat (of having ES5, ES6, etc. builds) and thus created a new NPM username for each of those packages, just to get @packageName/ESX.

So I think @ianobermiller's suggestion of a separate un-scoped package (e.g. `radium-es6') would be more appropriate.

[begincalendar]

UglifyJS can't minify ES6 code.

So if the proposed (separate build) strategy were to go ahead, UglifyJS would need to be replaced with something like babili.

[andrewfrankel]

Any updates on this? FWIW, radium breaks with basic jest testing in 'create-react-app' (https://github.com/facebookincubator/create-react-app) which might be a deal breaker on a current project.

[ryan-roemer]

Is there a .babelrc set of settings that would transform radium/src to a workable form for say node6+? I think we'd be potentially open to another transpile target.

[andrewfrankel]

@ryan-roemer

Not sure how helpful this is, but here's what the starter app uses by default: https://github.com/facebookincubator/create-react-app/blob/master/packages/babel-preset-react-app/index.js

The npm package has a simpler list to peek at: https://github.com/facebookincubator/create-react-app/blob/master/packages/babel-preset-react-app/package.json

[afrankel-sfdo]

@ryan-roemer as a quick test this appears to work for me:

Note: using uglifyjs-webpack-plugin 1.0.0-beta.2 which supports es6

.babelrc

{
  "plugins": [
    "add-module-exports",
    "transform-decorators-legacy"
  ],
  "presets": [
    ["env", {
      "targets": {
        "node": "current"
      }
    }],
    "stage-1",
    "react"
  ]
}

webpack

var webpack = require('webpack');
var UglifyJSPlugin = require('uglifyjs-webpack-plugin')
var merge = require('lodash.merge');

module.exports = merge({}, require('./webpack.config.js'), {
  output: {
    filename: 'radium.min.js'
  },
  plugins: [
    new UglifyJSPlugin(),
    new webpack.DefinePlugin({
      "process.env": {
        // Signal production mode for React JS and other libs.
        NODE_ENV: JSON.stringify("production")
      }
    })
  ]
});

[onpaws]

Radium is great and we want to use it, but at the moment Jest is blocked by this.

TypeError: Class constructor NavBar cannot be invoked without 'new'
      at new RadiumEnhancer (node_modules/radium/lib/enhancer.js:89:72)

Started with create-react-app, so @andrewfrankel you nailed it.

Is Radium compatible with Jest?

[andrewfrankel]

@onpaws correct, when radium was compiled using that setup the jest issues were resolved, but we've since abandoned radium given this and other reasons.

[ezintz]

You should look for another library since the maintainers sadly are not providing any good solution for this issue. I gave up last year.

Now there are plenty of libs that are not so bad, just use the big search engine (;

[ryan-roemer]

@andrewfrankel -- If you can create a repository with a minimal reproduction of the error + steps to see it, I'll jump on it.

@ezintz -- Apologies, we've closed / fixed / updated over 30 tickets in the past couple weeks and are working through our backlog. We've got a lot of different OSS projects that require a lot of attention (aka Victory, webpack-dashboard, nodejs-dashboard, etc.) and only so many folks of our own to do them.

We will eventually get to everything in our backlog here (and the other projects too), but anything the community can help with diagnosis _plus_ PR-wise always speeds things up!

[onpaws]

@ryan-roemer Minimal repo

git clone [email protected]:onpaws/RadiumErrorDemo.git
cd RadiumErrorDemo
yarn
yarn test

[onpaws]

Just one guy's two cents:
As you know create-react-app is pretty popular, and I assume you'd like to see Radium adoption increase. Thus it seems to me like a good use of time to strive for out-of-the-box compat.

It would be a big factor for my team. Right now we're at a crossroads between Radium and test coverage. We like Radium but it's a pretty significant issue for us right now

[ryan-roemer]

@onpaws -- Yeah, I'll jump on the repo (thanks!) when I get a chance. I'm currently working on the React@16 upgrade and there's some weird test stuff going on with ErrorBoundarys in DEV... This is next up for my Radium work.

Separate note for everyone -- There _is_ a babel config above that purportedly just works. You should be able to just do a custom webpack rule to build it in the meantime. (I assume CRA allows you to add custom rules to webpack config)

[onpaws]

Thanks for the update Ryan.
Good luck on the ErrorBoundary piece, it sounds like an interesting React improvement.

Quick aside regarding why CRA and custom rules aren't as obvious as one might like...
It seems the raison d'etre for CRA is to be "config free" so to speak.
It neatly abstracts/automates away important pieces of the stack, including the module bundler/webpack. Lovely for newcomers but can occasionally pose setbacks like today.

In fact a while ago, I recall reading a CRA GH Issue when the team was trying to keep the door open to moving away from webpack -- thus there is no supported path to a custom webpack config.

Of course it being open source, the community will find a way, and here's one option.
https://daveceddia.com/customize-create-react-app-webpack-without-ejecting/
NB: haven't personally tried yet...

[ryan-roemer]

Full exception stack for research notes:

TypeError: Class constructor App cannot be invoked without 'new'
    at new RadiumEnhancer (/Users/rye/scm/vendor/onpaws-RadiumErrorDemo/node_modules/radium/lib/enhancer.js:89:72)
    at constructClassInstance (/Users/rye/scm/vendor/onpaws-RadiumErrorDemo/node_modules/react-dom/cjs/react-dom.development.js:9760:20)
    at updateClassComponent (/Users/rye/scm/vendor/onpaws-RadiumErrorDemo/node_modules/react-dom/cjs/react-dom.development.js:10215:9)
    at beginWork (/Users/rye/scm/vendor/onpaws-RadiumErrorDemo/node_modules/react-dom/cjs/react-dom.development.js:10605:16)
    at performUnitOfWork (/Users/rye/scm/vendor/onpaws-RadiumErrorDemo/node_modules/react-dom/cjs/react-dom.development.js:12573:16)
    at workLoop (/Users/rye/scm/vendor/onpaws-RadiumErrorDemo/node_modules/react-dom/cjs/react-dom.development.js:12682:28)
    at HTMLUnknownElement.callCallback (/Users/rye/scm/vendor/onpaws-RadiumErrorDemo/node_modules/react-dom/cjs/react-dom.development.js:1299:14)
    at invokeEventListeners (/Users/rye/scm/vendor/onpaws-RadiumErrorDemo/node_modules/jsdom/lib/jsdom/living/events/EventTarget-impl.js:219:27)
    at HTMLUnknownElementImpl._dispatch (/Users/rye/scm/vendor/onpaws-RadiumErrorDemo/node_modules/jsdom/lib/jsdom/living/events/EventTarget-impl.js:126:9)
    at HTMLUnknownElementImpl.dispatchEvent (/Users/rye/scm/vendor/onpaws-RadiumErrorDemo/node_modules/jsdom/lib/jsdom/living/events/EventTarget-impl.js:87:17)
    at HTMLUnknownElementImpl.dispatchEvent (/Users/rye/scm/vendor/onpaws-RadiumErrorDemo/node_modules/jsdom/lib/jsdom/living/nodes/HTMLElement-impl.js:36:27)
    at HTMLUnknownElement.dispatchEvent (/Users/rye/scm/vendor/onpaws-RadiumErrorDemo/node_modules/jsdom/lib/jsdom/living/generated/EventTarget.js:61:35)
    at Object.invokeGuardedCallbackDev (/Users/rye/scm/vendor/onpaws-RadiumErrorDemo/node_modules/react-dom/cjs/react-dom.development.js:1338:16)
    at invokeGuardedCallback (/Users/rye/scm/vendor/onpaws-RadiumErrorDemo/node_modules/react-dom/cjs/react-dom.development.js:1195:27)
    at performWork (/Users/rye/scm/vendor/onpaws-RadiumErrorDemo/node_modules/react-dom/cjs/react-dom.development.js:12800:7)
    at scheduleUpdateImpl (/Users/rye/scm/vendor/onpaws-RadiumErrorDemo/node_modules/react-dom/cjs/react-dom.development.js:13185:19)
    at scheduleUpdate (/Users/rye/scm/vendor/onpaws-RadiumErrorDemo/node_modules/react-dom/cjs/react-dom.development.js:13124:12)
    at scheduleTopLevelUpdate (/Users/rye/scm/vendor/onpaws-RadiumErrorDemo/node_modules/react-dom/cjs/react-dom.development.js:13395:5)
    at Object.updateContainer (/Users/rye/scm/vendor/onpaws-RadiumErrorDemo/node_modules/react-dom/cjs/react-dom.development.js:13425:7)
    at /Users/rye/scm/vendor/onpaws-RadiumErrorDemo/node_modules/react-dom/cjs/react-dom.development.js:17105:19
    at Object.unbatchedUpdates (/Users/rye/scm/vendor/onpaws-RadiumErrorDemo/node_modules/react-dom/cjs/react-dom.development.js:13256:14)
    at renderSubtreeIntoContainer (/Users/rye/scm/vendor/onpaws-RadiumErrorDemo/node_modules/react-dom/cjs/react-dom.development.js:17104:17)
    at Object.render (/Users/rye/scm/vendor/onpaws-RadiumErrorDemo/node_modules/react-dom/cjs/react-dom.development.js:17129:12)
    at Object.<anonymous>.it (/Users/rye/scm/vendor/onpaws-RadiumErrorDemo/src/App.test.js:12:24)
    at Object.asyncFn (/Users/rye/scm/vendor/onpaws-RadiumErrorDemo/node_modules/jest-jasmine2/build/jasmine-async.js:68:30)
    at resolve (/Users/rye/scm/vendor/onpaws-RadiumErrorDemo/node_modules/jest-jasmine2/build/queueRunner.js:38:12)
    at Promise (<anonymous>)
    at mapper (/Users/rye/scm/vendor/onpaws-RadiumErrorDemo/node_modules/jest-jasmine2/build/queueRunner.js:31:21)
    at Promise.resolve.then.el (/Users/rye/scm/vendor/onpaws-RadiumErrorDemo/node_modules/p-map/index.js:46:16)
    at <anonymous>

[ryan-roemer]

Hi folks -- I'm working on the issue here: https://github.com/FormidableLabs/radium/compare/bug/es-node-classes

Right now, I'm trying to just create a regression test. My thought was to do a native, untranspiled Node test using the transpiled radium/lib code as source. To that end I have: https://github.com/FormidableLabs/radium/blob/f58bfebf3b4de4f3d1b71cfe7f2d0be4df7a29c4/test/enhancer-test.js#L15-L23

This is supposed to be a failing test, but it currently passes.

I would assume that this should expose the error because I'm using a Node native class and wrapping it with the Radium enhancer (the above is the same as wrapping in Radium()).

Can anyone see why I wouldn't be getting the same error? Would love someone to fork my branch and come up with a proper, failing regression test for native classes throwing when used with Radium. (Because weirdly it seems they _are_ working fine here...)

/cc @alexlande

[begincalendar]

@onpaws Thanks for the repo, but yeah the fact that CRA abstracts away the build config makes debugging this issue more difficult than necessary.

We need reproducible source AND build config to sort this out.
I would try to create one myself, but I can't recall the repo that led me to originally come across this issue.

[begincalendar]

@ryan-roemer
The issue with your test case is that your class does not define a render() method and this causes the input class to get wrapped into a function here.

So if you define a render() method in your inline class, you should find the issue gets reproduced.

[ryan-roemer]

@begincalendar -- Thanks! The regression test now fails:

$ yarn run test-node
yarn run v1.1.0
$ mocha 'test/**/*-test.js'


  Enhancer
    ✓ sets up initial state
    1) handles native ES classes


  1 passing (9ms)
  1 failing

  1) Enhancer handles native ES classes:
     TypeError: Class constructor Composed cannot be invoked without 'new'
      at new RadiumEnhancer (lib/enhancer.js:89:72)
      at Context.React.Component (test/enhancer-test.js:24:22)

... so I've got a good starting point to work on from here...

[ryan-roemer]

Ok folks, I've got a work-in-progress PR up at https://github.com/FormidableLabs/radium/pull/936

It works for very elementary cases (and the error repo @onpaws posted). But I'd love for folks to kick the tires in projects that are hopefully more complicated and ensure that all the parts of Radium are still working as intended.

Thanks!