[WIP] [Core] Setup umd build

[ANich]

Resolves #262

Hi first time contributor here,

npm run build:umd script creates a standalone MaterialUI.js in the build/umd dir. Likewise npm run build:min creates a minified version MaterialUI.min.js.

I tried including it in a script tag and the components were available under the MaterialUIobject, but I haven't tested it completely. Is this sufficient?

I included react-tap-event-plugin as an external, though I'm not sure if it has an external build. Should it remain?

• PR has tests / docs demo, and is linted.
• Commit and PR titles begin with [ComponentName], and are in imperative form: "[Component] Fix leaky abstraction".
• Description explains the issue / use-case resolved, and auto-closes the related issue(s) (http://tr.im/vFqem).

[mbrookes]

@ANich Nice work.

One thing I think is important, however we arrive at it, is allow the component imports currently defined in the examples to work as-is. This way we will be able to push them unmodified to codepen or similar via the API, without having to rewrite the imports (although we'll still need to take care of putting the theme on context).

@EloB did something similar to that here: #3997 (comment) (explanation is a couple of comments down).

Edit: I realised after I wrote that that it didn't make sense. 😄 And in this case, it looks like they're imported from the main index.js, which may be all that's possible, since naturally you can't have paths in a single module. Hmm...

[ANich]

Hey @mbrookes thanks!

Hmm, did you mean some way to get all of the components available under the window instead of a MaterialUI object? See 1f4165c , index.umd.js exports getMuiTheme and MuiThemeProvider

Related: is exporting all of these from index.umd.js fine? They are exposed as MaterialUI.getMuiTheme etc.

Edit: 'They' meaning everything, everything from src/index.js as well as src/styles/

[mbrookes]

Hmm, did you mean some way to get all of the components available under the window instead of a MaterialUI object?

Is that even possible? :)

However, if we're going to have to do some kind of inline code-mode on the docs site examples when they are pushed to the codepen API in order to get them working with a standalone build, it may not matter so much what the imports look like.

@EloB's approach at least means that the import resembles something that would work (albeit inefficiently without tree shaking) if you were to copy them from codepen into a local project:

import { Tabs, Tab } from 'material-ui';

[mbrookes]

@ANich Could you post a link to this working in codepen or jsfiddle or similar?

I uploaded the umd lib to a branch on my repo, and tried to access it via rawgit.com in a copy of @EloB's fiddle, but couldn't get it to recognise the imports.

[mbrookes]

Here's @EloB's version working with the Card example from the docs site.

To make the examples work, all we'd need to do is:

• replace the import directory 'material-ui/ComponentName' with 'material-ui' for any components used
• add the react-dom and react-tap-event-plugin imports,
• add a call to injectTapEventPlugin();
• and replace the export with:

ReactDOM.render((
React.createElement(ExampleName)
), document.getElementById('someId'));

Should be possible to do that on the fly, but worst case we can have a build script that converts the examples as part of the docs site build.

(Same in CodePen: http://codepen.io/mbrookes/pen/gMYZzV)

[ANich]

@mbrookes

Is that even possible? :)

No idea 😅

I'm not exactly sure what you mean by:

One thing I think is important, however we arrive at it, is allow the component imports currently defined in the examples to work as-is.

By as-is do you mean verbatim?

However, if we're going to have to do some kind of inline code-mode on the docs site examples when they are pushed to the codepen API in order to get them working with a standalone build, it may not matter so much what the imports look like.

@ANich Could you post a link to this working in codepen or jsfiddle or similar?

Sure thing: https://jsfiddle.net/jfr174ck/2/

I took the first example from the usage page and ran it through the babel repl.

This was my interpretation of what someone might write if they were using the umd build without babel or any other node technology in their workflow. That being said, React, ReactDOM must also be included separately and babel would have to be included separately from a CDN or js file to access imports or jsx.

Here's my rawgit URL

(Rawgit is pretty cool thanks for the new tool!)

[newoga]

@mbrookes With @ANich's latest push, we should have all of the components and style related files exported under index (in his new index.umd.js). So you should be able to import everything you need (including things like MuiThemeProvider) from material-ui or in another words (MaterialUi.MuiThemeProvider)

Also, in the umd build, react-tap-event-plugin is now bundled and injectTapEventPlugin() is automatically called (this is because there isn't a browser build for react-tap-event-plugin).

What do you think of this approach?

Should be possible to do that on the fly, but worst case we can have a build script that converts the examples as part of the docs site build.

I agree, sounds like a good build related activity. If the approach for making the browser build looks good, maybe we could merge this first. That way we can get it up in a CDN for our next release and start sorting out codebin integration.

[newoga]

@ANich Looks like there is some interesting behavior when we click the Button in your jsfiddle, getting some errors in the console. Not sure if it's code or build related yet...mmmm

[ANich]

@ANich Looks like there is some interesting behavior when we click the Button, getting some errors in the console. Not sure if it's code or build related yet...mmmm

@newoga Yeah you're right. Also for some reason, I couldn't reproduce the errors on Firefox.

For reference, these are the errors on Chrome:

react.js:19106 Uncaught Invariant Violation: addComponentAsRefTo(...): Only a ReactOwner can have refs. You might be adding a ref to a component that was not created inside a component's `render` method, or you have multiple copies of React loaded (details: https://fb.me/react-refs-must-have-owner).invariant @ react.js:19106ReactOwner.addComponentAsRefTo @ react.js:12530attachRef @ react.js:13343ReactRef.attachRefs @ react.js:13362attachRefs @ react.js:13172_assign.notifyAll @ react.js:839ON_DOM_READY_QUEUEING.close @ react.js:13064Mixin.closeAll @ react.js:16276Mixin.perform @ react.js:16223Mixin.perform @ react.js:16210_assign.perform @ react.js:13914flushBatchedUpdates @ react.js:14001Mixin.closeAll @ react.js:16276Mixin.perform @ react.js:16223ReactDefaultBatchingStrategy.batchedUpdates @ react.js:9919batchedUpdates @ react.js:13922ReactEventListener.dispatchEvent @ react.js:10898
MaterialUI.js:41689 Uncaught TypeError: Cannot read property 'componentWillLeave' of undefined

Maybe worth opening an issue about?

[mbrookes]

@ANich

This was my interpretation of what someone might write if they were using the umd build without babel or any other node technology in their workflow.

Are you suggesting this might be used outside of code play-pens? Material-UI isn't intended to be used as a monolithic lib, so to me the primary use-case is pretty clear, and we should be developing towards that, rather than a hypothetical user who don't want to use build tools. If their own code is so simple that it doesn't need to be modular, then dragging a 1.5MB library along for the ride would be crazy. 😜

@newoga

What do you think of this approach?

TBH I much prefer the resulting code in codepen using the alternate approach. It bears a much closer resemblance to the original examples, having ES6 imports, and will make it easier to go both directions, for example taking a modified example from codepen that reproduces a bug in a Material-UI component, and dropping it back into the docs site for further testing and development.

Edit: Add attribution.

[newoga]

@mbrookes This PR isn't about modifying examples, just creating the browser build of Material-UI (which is a stepping stone to using it for examples on sites like codebin and jsfiddle). How this is currently setup is pretty much how every other React project creates their browser build. It's up to the user to include additional dependencies like React, and babel if they want to use ES6.

Maybe I'm not understanding what alternative approach your are suggesting. Could you re-explain? 😄

[mbrookes]

@newoga

This PR isn't about modifying examples, just creating the browser build of Material-UI

To use with the examples. In codepen.

So how the build is generated, and what the resulting imports look like when used in a code play-pen is absolutely relevant.

Maybe I'm not understanding what alternative approach your are suggesting.

The one that results in a browser build that can be used like this:
#4342 (comment)

[newoga]

@mbrookes Your requirements (and codepen compatibility) is satisfied by this PR minus a few differences, which is why I'm confused what you're asking for?

replace the import directory 'material-ui/ComponentName' with 'material-ui' for any components used

That's done. All components and files underneath styles are now accessible under material-ui. This is being done in index.umd.js.

add the react-dom and react-tap-event-plugin imports,
add a call to injectTapEventPlugin();

That's fine, provided the user loads react and react-dom themselves. Call to injectTapEventPlugin() isn't necessary with this alternative build because it is included and ran for you. This is the only difference as far as I can see with what you suggested.

and replace the export with:

That's also not a problem.

[newoga]

The only problem I think we have is related to an error we're getting in some browsers (which I think is related to material-ui's use of react-addons-transition-group. We'll probably have to require react-with-addon.js browser build instead and make react-addons-transition-group external in the webpack build as well.

[mbrookes]

Your requirements (and codepen compatibility) is satisfied by this PR minus a few differences, which is why I'm confused what you're asking for?

No, it's me that's confused. 😜 Webpack is a black art (that I haven't studied), so I can only base my assessment on the end result, rather than the webpack config, which is why I asked for a working jsfiddle.

I should probably have linked to my codepen or jsfiddle rather than the comment, but the main difference between the two that I'm concerned about is having to use this:

<MaterialUI.RaisedButton label="Default" />

(or this:

const {RaisedButton} = MaterialUI;

<RaisedButton label="Default" />

)
instead of this:

import RaisedButton' from 'material-ui';

<RaisedButton label="Default" />

[mbrookes]

Ping @ANich / @newoga

[newoga]

@ANich Can you fix that react-addons-transition-group issue if you haven't done so already?

[mbrookes]

We also need to decide what the target directory should be - build is for the NPM package.

[newoga]

We also need to decide what the target directory should be - build is for the NPM package.

I agree. 👍 I've seen /dist used in another projects for this type of build.

[mbrookes]

@newoga Sounds good. Also, what about the object name? MUI?

[newoga]

@newoga Sounds good. Also, what about the object name? MUI?

I don't have a strong opinion about that one. I find most projects just pick the pascal case of their package name. But I wouldn't argue if the consensus was leaning towards something else.

[mbrookes]

If that's the norm, I'm fine with that.

[ANich]

If that's the norm, I'm fine with that.

@mbrookes I've left it as MaterialUI per Pascal Case

We also need to decide what the target directory should be - build is for the NPM package.

I agree. 👍 I've seen /dist used in another projects for this type of build.

I've changed the directory to dist and added it to the .gitignore,

See 342f497

@ANich Can you fix that react-addons-transition-group issue if you haven't done so already?

@newoga I'm not 100% sure where to go with this, I've tried using react-with-addons.js instead of the regular package and added react-addons-transition-group as an external in webpack. See: https://jsfiddle.net/jfr174ck/9/ Still investigating..

This might be useful, changing the order of the external scripts (MaterialUI.js before react-dom.js) changes the error but the animation still doesn't fire. https://jsfiddle.net/jfr174ck/8/

[newoga]

@ANich Interesting. I just noticed we also have react-addons-create-fragment as a dependency. Maybe that needs to be external too. I'll try your branch a little later and give it a try.

In package.json:

    "react-addons-create-fragment": "^15.0.1",
    "react-addons-transition-group": "^15.0.1",

[ANich]

@nathanmarks @newoga @mbrookes Updated the code 👍 https://jsfiddle.net/jfr174ck/12/

I did a rebase and pushed to my branch, do I need to anything else to get this merged?

[ricardopolo]

What is missing in this to be merged? :)

[ricardopolo]

The versions generated with this are going the version that will eventually go to CDNS?

[ANich]

Ping @nathanmarks @newoga @mbrookes

[Deraen]

Just tested this and seems like there is a problem:

MaterialUI.js:79 Uncaught TypeError: Cannot redefine property: MuiThemeProvider

MuiThemeProvider is exported both from index.js and styles/index.js so the UMD wrapper tries to export it twice, which fails.

Perhaps stuff from styles should be exported under different var (MaterialUIStyles) or under a property in the main var (MaterialUI.Styles)?

[neechange]

Can be used to generate rollup.js it

[vans163]

I tried this but I am getting errors when I load the module. Also npm install && npm run build:umd is broken as of 4 days ago since the react-tap-event-plugin updated to 2.0.1.

Same problem as @Deraen, how did you resolve it btw? I appended 2 to the MuiThemeProvider import in ./src/index.js.

If I load it as text/javascript it gives error, "Uncaught TypeError: Cannot redefine property: MuiThemeProvider" On line 79 of MaterialUI.js "Object.defineProperty(exports, key, {"

Also is it just me or are SvgIcons and FontIcons not working with this UMD build?

[nealeu]

These two targets gave errors for me. I changed them to:

"build:umd": "cross-env NODE_ENV=development webpack src/index.umd.js --output-filename dist/MaterialUI.js",
"build:min": "cross-env NODE_ENV=production webpack -p src/index.umd.js --output-filename dist/MaterialUI.min.js",

and the build ran correctly.

[vans163]

@nealeu Do icons and touch events work for you? Im thinking if I should just try and apply this umd build patch to the next branch, which removes reacttouchevents and uses onClick.

[nealeu]

@vans163 So far, yes. The only issue I've found so far is with un-docked Drawer. swipe to open and close work okay, but click on overlay to dismiss doesn't work. I'm adding an explicit close icon at the top of in the menu to deal with that.

[vans163]

@nealeu Good to know it can actually work. I will try to apply this to the 'next' branch and see if the switch to using onClick fixes the undocked Drawer issue.

EDIT: Still struggled with this, onTouchEvent was still broken. Using next branch nothing at all would render.

Ended up using this UMD build https://github.com/cljsjs/packages/tree/master/material-ui. It correctly applies the troublesome react-tap-event-plugin, and it correctly considers the SVG images plus other misc stuff that ANich missed.

[nealeu]

Hi,
I agree. It took me longer than you to hit problems, but Material UI is broken without onTouchTap (e.g. Snackbar action uses it and does not provide an onClick alternative).

My feeling on this is that Material UI should work fully with onClick so that it's desktop compatible.

I think a separate pull request is needed which is to use onClick in the places it is needed.

I'll start one.

[vans163]

@nealeu The 'next' branch I think is the one that uses onClick in most places, even though its called onTouchTap (for backwards compatibily as I understand it), it has no dependency on react-tap-event-plugin. But the next branch is totally undocumented and when I switch to it with my working code, it just rendered a blank page.

[nealeu]

@vans163 Ah thanks. I hadn't spotted that. I'll rebase off it and see how I go.

[vans163]

@nealeu Be careful its missing components compared to master (such as stepper), I think the pull req you opened is more accurate. next seems to be still in heavy development.

[oliviertassinari]

I'm closing this PR as #262 has been resolved. Thanks for your help guys.