Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Problems arise when using meteor add react with a browserified React package like material ui #11

Closed
grigio opened this issue Jun 9, 2015 · 40 comments

Comments

@grigio
Copy link
Contributor

grigio commented Jun 9, 2015

I tried to meteor add react and add material-ui via cosmos:browserify but it seems the context is undefined.

No problem with react and material-ui added via cosmos:browserify

@pronevich
Copy link
Contributor

I have the same issue. Early I tried reactjs:react package with https://github.com/ukabu/material-ui but as a result I got the same issue as mentioned above.Then I have tried https://github.com/mrphu3074/react-material-ui/ and it's works fine, this package exports all components globally.

Now with react-packages I do it right and add material-ui via npm and cosmos:browserify but I can't use components because context is undefined.

@stubailo
Copy link
Contributor

stubailo commented Jun 9, 2015

The problem involves those packages pulling in their own browserified
React; we end up with two reacts, and they can't share the context. I'm
thinking about ways to fix this.
On Tue, Jun 9, 2015 at 7:17 AM Alexey Pronevich notifications@github.com
wrote:

I have the same issue. Early I tried reactjs:react package with
https://github.com/ukabu/material-ui but as a result I got the same issue
as mentioned above.Then I have tried
https://github.com/mrphu3074/react-material-ui/ and it's works fine, this
package exports all components globally.

Now with react-packages I do it right and add material-ui via npm and
cosmos:browserify but I can't use components because context is undefined.


Reply to this email directly or view it on GitHub
#11 (comment)
.

@grigio
Copy link
Contributor Author

grigio commented Jun 9, 2015

@stubailo I've seen you removed cosmos:browserify as dependency and bundled ReactRouter in lib/ what is the alternative to install browserified dependencies like ReactRouter, material-ui,.. ?

Could and approch be to install the browserified dependencies via Npm.depends({..}) or an ad-hoc package support ?

Browserify.depends({
  'react-router':'0.13.3',
  'material-ui':'0.8.0'
});

@stubailo
Copy link
Contributor

stubailo commented Jun 9, 2015

@grigio no, that doesn't help. The problem is that as soon as you install a package that depends on react, you get a second copy of React.

We're going to implement proper NPM/Browserify support soon, but probably not in the next few weeks.

@grigio
Copy link
Contributor Author

grigio commented Jun 9, 2015

I mean Material UI works if you install React via cosmos:browserify and the rest a la Meteor way.
You removed cosmos:browserify as dep, is there something wrong with it?

@stubailo
Copy link
Contributor

stubailo commented Jun 9, 2015

Ah, you're saying you could use the react npm package rather than meteor add react. That should probably work. Does that end up loading the right version of React for production/development?

Also, if you load React over NPM, then the MeteorDataMixin still works, right?

There's nothing particularly wrong with cosmos:browserify, but I wasn't happy with the lack of configuration, for example. I decided to defer worrying about that until later.

@stubailo stubailo changed the title React: this.context.muiTheme is undefined Problems arise when using meteor add react with a browserified React package like material ui Jun 9, 2015
@grigio
Copy link
Contributor Author

grigio commented Jun 10, 2015

Yes, you can't choose the React production/development but the rest works pretty well. I wrote a post about it.

@stubailo
Copy link
Contributor

@grigio try this: https://github.com/meteor/react-packages/blob/master/docs/client-npm.md

The new version of cosmos:browserify supports transforms, so you can use exposify to use the Meteor React package.

@grigio
Copy link
Contributor Author

grigio commented Jun 22, 2015

I tried a few alternative integrations, but I'm not sure I can use meteor-reactinstead of npm-react to run npm-material-ui at package level.

Or maybe you mean to load meteor-react, exposify Reactand then load npm-material-ui all the meteor app without private packages?

=> Started proxy.                             
=> Started MongoDB.                           
W20150622-10:16:52.122(2)? (STDERR)           
W20150622-10:16:52.124(2)? (STDERR) /Users/grigio/.meteor/packages/meteor-tool/.1.1.3.ttennc++os.osx.x86_64+web.browser+web.cordova/mt-os.osx.x86_64/dev_bundle/server-lib/node_modules/fibers/future.js:245
W20150622-10:16:52.125(2)? (STDERR)                         throw(ex);
W20150622-10:16:52.125(2)? (STDERR)                               ^
W20150622-10:16:52.350(2)? (STDERR) TypeError: Cannot read property 'PropTypes' of undefined
W20150622-10:16:52.351(2)? (STDERR)     at Object.<anonymous> (lib/node_modules/material-ui/lib/ripples/focus-ripple.js:17:1)
W20150622-10:16:52.351(2)? (STDERR)     at Object.../mixins/style-propable (lib/node_modules/material-ui/lib/ripples/focus-ripple.js:92:1)
=> Modified -- restarting.                    
browserify-deps: updating npm dependencies -- react, material-ui...

events.js:72
        throw er; // Unhandled 'error' event
              ^
Error: Cannot find module 'exposify' from '/Users/grigio/+persistant/Code/testing/react-materialui-demo/packages/browserify-deps/.npm/package'
    at /Users/grigio/.meteor/packages/cosmos_browserify/.0.4.0.1v8md0x++os+web.browser+web.cordova/plugin.CosmosBrowserify.os/npm/CosmosBrowserify/node_modules/browserify/node_modules/resolve/lib/async.js:46:17
    at process (/Users/grigio/.meteor/packages/cosmos_browserify/.0.4.0.1v8md0x++os+web.browser+web.cordova/plugin.CosmosBrowserify.os/npm/CosmosBrowserify/node_modules/browserify/node_modules/resolve/lib/async.js:173:43)

@grigio
Copy link
Contributor Author

grigio commented Jun 22, 2015

@stubailo I tried to integrate Material UI at app-level

meteor add React

it loaded and present before:

// lib/app.browserify.js
mui = require("material-ui");
// lib/app.browserify.options.json
{
  "transforms": {
    "exposify": {
      "global": true,
      "expose": {
        "react": "React"
      }
    }
  }
}
// packages.json
{
  "exposify": "0.4.3",
  "material-ui": "0.8.0"
}

the UI is loaded but it is unusable, it seems React has been loaded twice

// inside inspector
Uncaught Error: Invariant Violation: addComponentAsRefTo(...): Only a ReactOwner can have refs. This usually means that you're trying to add a ref to a component that doesn't have an owner (that is, was not created inside of another component's `render` method). Try rendering this component inside of a new top-level component which will hold the ref.
Exception from Tracker recompute function:
debug.js:41 TypeError: Cannot read property 'offsetHeight' of null
    at _positionDialog (app.browserify.js?e5806da9989a8fae75d6ecb6a8f5de6de6080f05:2520)

@stubailo
Copy link
Contributor

I have created a minimal material UI example here: https://github.com/meteor/react-packages/tree/master/examples/material-ui-example

Can you help me construct one that fails in the way you describe @grigio?

@grigio
Copy link
Contributor Author

grigio commented Jun 22, 2015

@stubailo Your example seems to work, but if I replace

var RaisedButton = mui.RaisedButton;

with

var DatePicker = mui.DatePicker;

and

<RaisedButton label="Default" />

with:

<DatePicker />

It doesn't work anymore:

Uncaught Error: Invariant Violation: addComponentAsRefTo(...): Only a ReactOwner can have refs. This usually means that you're trying to add a ref to a component that doesn't have an owner (that is, was not created inside of another component's render method). Try rendering this component inside of a new top-level component which will hold the ref.

To get the previous error you can replace on your example material-ui-example.jsx with this and you reproduce the error.

@stubailo
Copy link
Contributor

Thanks for the repro! I'll look into it.

@stubailo
Copy link
Contributor

@grigio confirmed, my transform doesn't work in this case because material-ui uses require("react/addons") everywhere instead of require("react").

@stubailo
Copy link
Contributor

This was the result of this issue: thlorenz/exposify#15

As soon as exposify is updated, I'll update the guide and Material UI should work.

@grigio
Copy link
Contributor Author

grigio commented Jun 23, 2015

Nice, how did you find/debugged that? the errors seems doesn't seem related

@stubailo
Copy link
Contributor

The problem is that material-ui uses require("react/addons") all over the place instead of require("react").

So you still end up with duplicates of React.

@stubailo
Copy link
Contributor

@grigio this should work now - check the example: https://github.com/meteor/react-packages/tree/master/examples/material-ui-example

You just need to delete your .npm folder and re-build dependencies, so that you get replace-requires@1.0.3:

@pronevich
Copy link
Contributor

I have tested this example. The UI is rendered and there are no errors, but actions (clicks) don't work. For example, I can't open DatePicker dialog and etc.

@stubailo
Copy link
Contributor

I guess I don't really know how it's supposed to work - I thought maybe I was using it wrong or something.

@stubailo stubailo reopened this Jun 25, 2015
@stubailo
Copy link
Contributor

Weird - I just ran the example, and the date picker is actually rendered to the DOM - it just doesn't show up. So I don't think it's actually a problem with Browserify, it's something else.

@pronevich
Copy link
Contributor

There is nothing wrong with example code. I tested it also in another react + meteor solution by grigio and everything works fine.

@stubailo
Copy link
Contributor

@pronevich can you link to that code please? I want to see what it does.

@stubailo
Copy link
Contributor

It seems to be an issue with the touchTap event: https://github.com/callemall/material-ui/blob/1790591cc2051b7c6c458cf7afc3271007b21c5c/src/date-picker/date-picker.jsx#L131 (that's the event that actually opens the dialog)

I think to enable it you need the touch tap plugin: https://github.com/zilverline/react-tap-event-plugin

I've tried installing that plugin, and it doesn't work yet - I might be doing something wrong.

@stubailo
Copy link
Contributor

Ok, that's the issue - that plugin doesn't work. Going to look into this.

@grigio
Copy link
Contributor Author

grigio commented Jun 26, 2015

I think it's a problem in meteor/browserify/exposify, I tried to load injectTapEventPlugin = require("react-tap-event-plugin"); as it should but it an empty function coming from here.
It worked in my example without exposify https://github.com/grigio/meteor-react-material-ui-demo

@stubailo
Copy link
Contributor

Yeah, I know what the issue is, fixing it today.

@stubailo
Copy link
Contributor

OK, I have now definitively solved this issue with a brand new Browserify transform called externalify. See how to use it here: http://react-in-meteor.readthedocs.org/en/latest/client-npm/#4-configure-browserify-and-transforms-in-appbrowserifyoptionsjson

@grigio
Copy link
Contributor Author

grigio commented Jun 26, 2015

I confirm just tried #22 , thanks @stubailo

@KristerV
Copy link

I still get the same error mentioned way up there.

Uncaught Error: Invariant Violation: addComponentAsRefTo(...): Only a ReactOwner can have refs. This usually means that you're trying to add a ref to a component that doesn't have an owner (that is, was not created inside of another component's `render` method). Try rendering this component inside of a new top-level component which will hold the ref.

I am currently using externalify, just like @stubailo's last link suggests (and how the react-package examples suggest). The only difference is that I'm trying to make a package out of MUI.

You can see my very small package here. It's also got a mini app for testing - just clone the repo and run meteor in the "demo" dir.

@stubailo stubailo reopened this Jul 17, 2015
@lourd
Copy link

lourd commented Jul 17, 2015

#61 discusses this is as well

@stubailo
Copy link
Contributor

Reopened primarily because there is a suspicion it doesn't work inside packages. I'll try it out later and report back.

@KristerV
Copy link

okay.. uhmm.. thanks to mrphu3074 I managed to get my package working by simply adding react as a dependency to the package. I do not know why the "owner, parent context" error does not appear anymore, this makes no sense bu it works..

I've also updated my package so you can have a try if you like. But it seems that we have a material-ui package now so this issue can be closed as far as I'm concerned.

@stubailo
Copy link
Contributor

OK, will close again; if it comes up again we can revisit it. Eventually we will have support for NPM packages without cosmos:browserify, and then we can solve this issue once and for all.

@KristerV
Copy link

you forgot to close. Could you link us to whatever this NPM fix is going to be? so I can keep an eye out.

@stubailo
Copy link
Contributor

Sorry, there isn't anywhere to link to yet!

@vdavid
Copy link

vdavid commented Dec 3, 2015

For me (and Meteor 1.2.1), the above solution did not work. For material-ui, this Meteor package helped me, it works flawlessly: https://atmospherejs.com/markoshust/material-ui

@SylarRuby
Copy link

I almost went back to Rails. Thank you @stubailo for: https://github.com/meteor/react-packages/blob/master/docs/client-npm.md

@SachaG
Copy link

SachaG commented Jan 20, 2016

I'm running into the same issue… I must be doing something wrong with the way I set up my dependencies? Here's the repo: https://github.com/meteor-utilities/Meteor-Griddle

@stubailo
Copy link
Contributor

@SachaG can you post a separate issue? This one is a bit long and about material UI.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

8 participants