Broken export when using Webpack 2 (React.createElement: type should not be null)

[catamphetamine]

In 3.1.0 the export is not broken: typeof require('react-helmet') === 'function'.
In 3.2.0 it is broken: typeof require('react-helmet') === 'object'.
The error is "Warning: React.createElement: type should not be null, undefined, boolean, or number. It should be a string (for DOM elements) or a ReactClass (for composite components). Check the render method of Layout."
The cause is that v3.2.x of ReactHelmet is { default: function ReactHelmet() {} } when imported in Webpack 2.

[mbifulco]

Just spent an entire day with @Bonitis trying to track down an issue that has this at its root. Good find, @halt-hammerzeit!

[Bonitis]

+1

[catamphetamine]

@mbifulco @Bonitis Yeah, this was a nontrivial one. Took me almost half an hour.

Fuck yeah, let do a circle jerk like

[andykenward]

Looking into this issue. Not having it with "webpack": "2.1.0-beta.22" myself in a project. So it may be the babel setup

[catamphetamine]

@andykenward I'm having it with 2.1.0-beta.22.
If it's babel setup then it's react-helmet's babel transpilation setup.

[andykenward]

Are you using the commonjs or es version in your project?

react-helmet/package.json

Lines 5 to 7 in f0b971c

	"main": "./lib/Helmet.js",
	"module": "es/Helmet.js",
	"jsnext:main": "es/Helmet.js",

As you need to set your babel config like this if using es

https://github.com/andykenward/bob/blob/develop/package.json#L47-L57

[catamphetamine]

@andykenward Why wouldn't I use "es2015 modules"?
Isn't it the whole point of the Brave New Future where code is automatically Tree-Shaken and the bundle size is reduced and Common.js is deprectated and obsolete.

[catamphetamine]

It works if I remove jsnext:main entry from react-helmet/package.json by the way (as you've mentioned).
But this is not a workaround since no one should tamper with 3rd party package.json.
My current workaround is:
https://github.com/halt-hammerzeit/react-isomorphic-render/blob/master/source/webpage%20head.js

[andykenward]

@halt-hammerzeit that is why you set it to false. As the babel preset 2015 transform the modules to commonjs by default.

Check the docs. Under Options
https://babeljs.io/docs/plugins/preset-es2015/

modules - Enable transformation of ES6 module syntax to another module type (Enabled by default to "commonjs").
Can be false to not transform modules, or one of ["amd", "umd", "systemjs", "commonjs"]

[catamphetamine]

@andykenward Well, my babel-loader is set up to clearly not perform any transpiling on node_modules folder

            {
                test    : /\.js$/,
                include :
                [
                    path.resolve(frontend_root_folder, 'code')
                ],
                loader: 'babel-loader'
            }

And I don't know how all this thing with Webpack and jsnext:main even works and there are no clues on this on the internets (the usual Webpack issue).
I suppose it kinda magically transpiles those jsnext:main modules by itself, possibly using Babel.
I'll ask them there:
webpack/webpack#2902 (comment)

[catamphetamine]

So, it seems like a non-react-helmet issue so I'm closing this, I guess.
Yeah, because jsnext:main was added in 3.2.x release and it's causing the bug.
More like Webpack issue.

[catamphetamine]

@andykenward So why do you set your "es2015"."modules" to false? Do you run your code only in ES6-compatible web browsers and ignore the old browsers?

[andykenward]

@halt-hammerzeit no i compile to ES5 javascript. That is why you use babel in the first place. To compile ES6/ES2015 code to ES5.

By setting the modules to false you stop babel from doing a transform of modules to commonjs. Which is the default behaviour for the Babel ES2015 preset.

read the doc for transform-es2015-modules-commonjs

Babel 6 Changes
Babel 6 changed some behavior by not doing module.exports = exports['default'] anymore in the modules transforms.

There are some caveats, but you can use babel-plugin-add-module-exports, so that updating to Babel 6 isn't a breaking change since users that don't use ES modules don't have to do require("your-module").default.

However, it may not match how Node eventually implements ES modules natively given the the current proposal.

And if you want webpack 2 to be able to go into any npm packages that are built with support for ES6/ES2015 version and tree shake those then you need to stop the modules transform. Otherwise you might aswell use the ES5 versions of the npm packages

There is a blog post about Tree-shaking with webpack 2 and Babel 6. Explaining this more. But the babel setup is outdated and you can now just set modules to false. Since version v6.13.0 of babel.

[andykenward]

Oh but you may need to use the babel-polyfill in some cases for older browsers. eg < IE 11

[catamphetamine]

@andykenward Well, okay, though I didn't understand a thing about all this alchemy.
But thanks for the explanations anyway.
This topic seems to be overly complicated (as all webpack-babel things are).

So, now I understand, that what modules: false does is it converts the module type (not the module source itself, just the module type) into something like commonjs, etc.
https://babeljs.io/docs/plugins/preset-es2015/
By default it converts modules to commonjs which means export default function Helmet() {} is made into module.exports = { default: function Helmet() {} } which now clearly explains the source of the bug.
By turning modules to false ALL modules aren't converted to commonjs which means by fixing react-helmet it can potentially break all the other existing modules which worked fine before.
Or it may not.
I don't bother right now since my temporary workaround works for the time being, while the folks who're working on this topic finally make an acceptable and simple solution which works out-of-the-box without any further tweaking and hacking around.

[cwelch5]

@halt-hammerzeit Apologies for the confusion with the babel setup. We are looking into seeing how we can fix this. Just want to be clear, you were using this library @ v3.1.0 with no problems and now with v3.2.1 your webpack build automatically picks up the module, which is the es version?

@andykenward We would prefer not to have people add special babel configuration to consume the library properly. I thought that adding the jsnext:main and module in the package.json created a purely opt-in situation. Will other users, besides Webpack users automatically get the es version?

[dlebedynskyi]

@cwelch5 3.1.0 work fine. issue only for ^3.2.0.
It would be nice to keep it as it is without special babel configs since it will be constant repeating issue

[catamphetamine]

@cwelch5 Yeah, I added jsnext:main to all my libraries because I also thought it was an opt-in feature. They seem to work by the way so I'm unsure why jsnext:main broke with 3.2.x of react-helmet. Yes, it picks jsnext:main because if I erase it from package.json then no error happens.

[andykenward]

@cwelch5 The issue is that the ES6 compiled version that is in es folder actually sticks to the standards for export default.

There are a few articles about this

Misunderstanding ES6 Modules, Upgrading Babel, Tears, and a Solution

ES6 Modules

I suggest we follow how redux and react-router do their exports without using export default on the entry files.

for example redux is

export {
  createStore,
  combineReducers,
  bindActionCreators,
  applyMiddleware,
  compose
}

and you import in your project as

import {createStore} from 'redux'

So then you would have to do import { Helmet } from 'react-helmet';

Or i can just compile the es version with commonjs exports not making it ES6

Let me know either way and I can do a pull request in a bit.

This shouldn't effect people using the ES5 version

[doctyper]

@andykenward That is a major change to the consumption API, which we should not do on a minor bump.

[andykenward]

@doctyper perhaps it would be best to remove module and jsnext:main from

https://github.com/nfl/react-helmet/blob/master/package.json#L6-L7

Then wait for a major release to implement it

[andykenward]

See pull request #197

[cwelch5]

Apologies for the headache on this. This is now fixed in v3.2.2

[tusbar]

@cwelch5 @andykenward are there any plans to revert this change and expose ES6 modules? :)

There have been a few major releases since then!

[tmbtech]

#395 <- uses rollup in instead of webpack to export the modules. This will allow for ESM exports.