ReactMDL.js bundle: "react" or "react-dom" instead of require('React') or require('ReactDOM')

[PavelPZ]

Is it possible to have this header in installed node_modules\react-mdl\out\ReactMDL.js file?

(function webpackUniversalModuleDefinition(root, factory) {
    if(typeof exports === 'object' && typeof module === 'object')
        module.exports = factory(require("react"), require("react-dom"));
    else if(typeof define === 'function' && define.amd)
        define(["react", "react-dom"], factory);
    else if(typeof exports === 'object')
        exports["ReactMDL"] = factory(require("react"), require("react-dom"));
    else
        root["ReactMDL"] = factory(root["react"], root["react-dom"]);
})(this, function(__WEBPACK_EXTERNAL_MODULE_3__, __WEBPACK_EXTERNAL_MODULE_14__) {```

```require("react")``` etc. seems to be the standard (see e.g. react-dom.js header) and I have problem with using ReactMDL.js in my NPM free deplyment.

[tleunen]

React and react-dom are not inside the bundle, you have to require them before including reactmdl ;)

You can use these urls: https://npmcdn.com/react@0.14.7/dist/react.min.js and https://npmcdn.com/react-dom@0.14.7/dist/react-dom.min.js

[PavelPZ]

Yes, I know, but react-dom.min.js uses require("react"), your node_modules\react-mdl\out\ReactMDL.js uses require("React").

So SystemJS module loader needs to map both react and React to the same module bundle. I am working without NPM, Babel, wabpack etc., just Typescript.

But forget for it...

[tleunen]

Oh I see.. Thanks for the clarification. I didn't get that at first.

I guess it's because of these lines: https://github.com/tleunen/react-mdl/blob/master/webpack.config.js#L3-L4

Feel free to submit a PR to fix the import names :)

[PavelPZ]

I tried something at my react-mdl fork.

[Download]

This change breaks including ReactMDL from a regular script tag.
See #298

[Download]

react and react-dom are the module names. React and ReactDOM are the names of the externals. This is reflected in webpack's config:

module.exports = {
  externals: {
    react: 'React',
    'react-dom': 'ReactDOM'
  },

This snippet of config is mapping the module react to the external React, which is expected to be found on the root object (which is this, which is window). This config was good before it was changed. These are externals, because ReactMDL does not get to decide what to fill in here. It was decided by Facebook when they chose react as their module name to publish on NPM and React as the name to use for publishing to the window object. All this config does is inform webpack about this.

[tleunen]

#272
Indeed, kinda make sense... So are the require('React') and require('ReactDOM') expected in the header of the output file?

[tleunen]

I'll revert the specific commit.

@PavelPZ I don't know your initial issue, but maybe this ticket will help you webpack/webpack#1275

[Download]

Indeed, kinda make sense... So are the require('React') and require('ReactDOM') expected in the header of the output file?

I don't think so actually. It is my understanding that require is supposed to accept the module name (lowercase), whereas the 'import' from the window object is supposed to use the uppercase name... I'm not sure what (if any) is the right term for that name... 'global name' maybe?

[tleunen]

In v1.4.3 for example, we had this header:

(function webpackUniversalModuleDefinition(root, factory) {
    if(typeof exports === 'object' && typeof module === 'object')
        module.exports = factory(require("React"), require("ReactDOM"));
    else if(typeof define === 'function' && define.amd)
        define(["React", "ReactDOM"], factory);
    else if(typeof exports === 'object')
        exports["ReactMDL"] = factory(require("React"), require("ReactDOM"));

So I'm guessing if someone wants to use the bundled file, with their own code, they would need to configure their webpack to say window.React should be used when require('react') is done, right?
I'm just unsure about these lines above. To me, it should be the name in lowercase...

This following like is ok though, since root is the window.
root["ReactMDL"] = factory(root["React"], root["ReactDOM"]);

Our config looks ok now so it should be fine, but still I don't really understand why it's these names

[Download]

Yeah I see what you mean. I agree that I would also expect the require lines to use the lowercase module name... I know that webpack is inserting it's own require so maybe it's got something to do with that?

[tleunen]

I don't know :/
This seems to be the real require function, not the webpack one __webpack_require__.
But it looks like are forced to use the same name for all cases.
Another example of this would be with ReactMDL. ReactMDL is the name exported on "window", but the package name is still react-mdl.

[Download]

Yeah you are right it must be the real require... But still it works? Strange. I have to admit that I find webpack's config options very confusing at times. Maybe the best thing would be to just ask a question on the webpack issue tracker? Sokra is a very helpful guy I bet he is willing to help. He helped me in the past with questions I had.

For comparison, here is React DOM:
https://npmcdn.com/react-dom@0.14.2/dist/react-dom.js

As you can see, it is doing what we would expect:

module.exports = f(require('react'));

and

g.ReactDOM = f(g.React);

However, it looks like this code was generated by another tool, or handwritten...

[Download]

I think your peerDependencies need an update BTW:

"peerDependencies": {
    "react": "0.14.x || ^15.0.0-rc",
    "react-dom": "0.14.x || ^15.0.0-rc"
  },

I think that NPM sees 15.0.0 as incompatible with 15.0.0-rc, though again I'm not sure.

[Download]

It seems you can specify aliases per output target:

externals: {
  "react": {
    root: 'React',
    commonjs2: 'react',
    commonjs: 'react',
    amd: 'react'
  }
}

See
http://stackoverflow.com/questions/34252424/webpack-umd-lib-and-external-files

[tleunen]

For the peerDeps, it should be ok, I used this tool to confirm http://semver.npmjs.com/

Good news for the output targets. I'll give it a try when I get some time :) Thanks again @Download

[Download]

@tleunen I'm preparing a PR for it

[Download]

@tleunen Could you re-open this issue?