jedwards1211 / meteor-webpack-react

(DEPRECATED) use jedwards1211/crater instead
https://github.com/jedwards1211/crater
374 stars 75 forks source link

Meteor specific issues #8

Open AdamBrodzinski opened 9 years ago

AdamBrodzinski commented 9 years ago

I think it would be a good idea to keep track of known issues with Meteor compatibility for new users that come across this. I'll try to knock these off as I have time. Feel free to close if this is unwanted.

jedwards1211 commented 9 years ago

Do you mean react-router or is there something called reaktor router? And I assume you're talking about loading these libs through Meteor packages? If you need to use React loaded via a Meteor package you can always make a script that exports React from whatever location the Meteor package puts it, and put the following in your Webpack config:

{
  resolve: {
    alias: {
      'react$': path.resolve(__dirname, 'getWindowReact.js'),
      'react/addons$': path.resolve(__dirname, 'getWindowReact.js'),
    }
  }
}

Only thing I don't remember is how to give the bundle access to the React variable, is it package scoped or does it get put in window.React?

jedwards1211 commented 9 years ago

And yeah I wouldn't expect using FlowRouter with this setup to be easy, I wasn't going to even try going down a path like that, but definitely share your insights with everyone if you succeed.

AdamBrodzinski commented 9 years ago

Oh cool I might try out that later. It might be easier to just use the react package and resolve it like you mentioned. It just exports it to window.React on client and global.React on the server. Generally every Meteor package exposes one namespace unless they intentionally leak others.

Would this solve the issue of having a different dev/prod version of React? (I vaguely remember something about that being swapped in .prod). They take care of swapping out the dev/prod version when running meteor --production or meteor build.

Reaktor Router is a facade over FlowRouter that looks a lot like react-router, without the nested UI. However for this upcoming project the app is much more like spotify than 'pages' so i'm going to def. use react-router for that. I'll get FlowRouter working sooner or later :smile: It seems like it was looking for the / route before it was defined. I only spent 10 mins on it though.

So far very little issues. I'm going to setup Velocity testing today. I'm assuming the velocity tests will need to be put in the meteor folder because they ultimately need to be dropped into a specific tests/jasmine/integration/foo.js directory. Maybe fire up a 3rd webpack?... just kidding lol #yolo

I'm already using Karama outside of Meteor for React unit tests so that should just drop right in :+1:

jedwards1211 commented 9 years ago

Actually I'm going to just put this feature into the project right now (getting React via this alias) I was mistaken about the prod React build thing -- one can build the prod version of React from the npm package by defining process.env.NODE_ENV to production and using UglifyJS. But making this skeleton automatically work with other Meteor packages would be good.

AdamBrodzinski commented 9 years ago

:+1: Cool. I'm working on implementing Meteor settings into the build step too. They're working really well. It even does a full reload when the settings file change. Meteor settings is used for things like setting up S3 keys for dev and production or public keys for the client.

jedwards1211 commented 9 years ago

Uhoh, bad news. So react-hot-loader requires react/lib/ReactMount, which ends up loading basically another copy of react, and it doesn't work with this aliasing scheme. So unfortunately using react-hot-loader together with the react-runtime meteor package is not recommended, and I won't be making this change. I will add a note in the README though. Personally, I'd rather not use any such meteor packages that don't fit neatly into the react/npm ecosystem.

jedwards1211 commented 9 years ago

I decided I'll make a branch later that uses react-runtime and no react-hot-loader. That way people will have both styles available.

AdamBrodzinski commented 9 years ago

Ah, bummer. I don't think it'll be too big of a deal. The worst case scenario is you can fork the package and add an imply config so that it will just assume that React is loaded instead of pulling in a specific verison.

AdamBrodzinski commented 9 years ago

btw.. I think having the react-hot-loader should be the default one. What do you think? For me that was one of the biggest perks (my current prod app takes 8 seconds to do a reload :frowning: ) I can also see how not having react-runtime will cause issues. My guess is people who are going to use this are ok with tinkering.

Just spitballing here but what if we create a Meteor package that does an NPM require for ReactMount, and exposes a global that we can resolve in webpack. We can set a debug flag so it's never used in prod (only dev mode).

It sounds like we couldn't do the above because any regular 'react' requires would use the NPM version instead right?

jedwards1211 commented 9 years ago

@AdamBrodzinski wait a minute...I wasn't right about that at all! All we have to do is bridge in the other direction, sticking the React loaded by Webpack into window.React, then any Meteor packages can get it as they normally would. There's some kind of way to exclude the react-runtime package right? I'll have to look into it tomorrow.

AdamBrodzinski commented 9 years ago

Hmmm, you could leave out the runtime and just include the mixin and that would work for anything that the user creates. However if another packages requires react or reactjs:react without a weak dep. then it will pull in another copy regardless.

If we could make a Meteor package that just pulls in ReactMount then we could use react and avoid the Meteor packages pulling in a 2nd copy without the user knowing. It would be kind of a pain to keep it synced but I think it might be the lesser of two evils?

Also i'm not sure if this is the same ReactMount but there's a separate one on NPM https://www.npmjs.com/package/react-mount that might be useable to alias/resolve react/lib/ReactMount to react-mount?

At any rate the developer experience will be 100x better if we can figure out how to use the Meteor React package. It's not a big deal to me but I can foresee people getting confused and frustrated.

jedwards1211 commented 9 years ago

Unfortunately that react-mount isn't the same thing. I think it's possible to put forks of the react and reactjs:react packages into the project that don't actually bring in the React library, and then any packages that depend on those will just use the local versions, so it should be frictionless.

Think about what a problem it is that a lot of Meteor packages were created solely to pull in some 3rd-party library, and in this case there are two incompatible ones doing the same thing that other packages might depend on. I'm really hoping Meteor eventually turns into something you just npm install, so that all 3rd-party Meteor packages just depend on Meteor and React or whatever else in their package.json.

I think we should just accept that there will be a bit of confusion and frustration when making a Frankenstein of tools like this -- to me it's still less frustration than trying to use React the way Meteor Development Group wants us to :)

AdamBrodzinski commented 9 years ago

I think we should just accept that there will be a bit of confusion and frustration when making a Frankenstein of tools like this

Agreed! I look at it like a fussy Lamborghini... it doesn't always run the way you want and the trim may fall off but once it's running it's an animal :laughing:

I'm really hoping Meteor eventually turns into something you just npm install, so that all 3rd-party Meteor packages just depend on Meteor and React or whatever else in their package.json

Yep same here. I'm hoping that when they do release ES6 modules it's going to be based on strictly NPM. I wouldn't mind the current atmosphere/packaging system to be deprecated. IMHO you could have all new > 1.3 packages be NPM, older legacy packages could still load in globally as they do today and when 2.0 rolls around remove the deprecated global packaging.

At any rate once we get these smoothed out i'll get some of the core devs to have a serious look at this since it can be used as a sandbox/strawman for the future Meteor ES6 modules.

grigio commented 9 years ago

Also wrapping any npm & deps is frankenstain.. I hope mdg and community will agree in a structure like this asap

jedwards1211 commented 9 years ago

Cool, I'm glad to hear you guys share my concerns. I'm sure a lot of MDG insiders have been thinking about this for awhile too. Sometimes I thought that since (as far as I've heard?) Meteor is backed by venture capital, they're using the package system to get people locked into their ecocsystem kind of like Microsoft does, but probably they're wiser than that.

grigio commented 9 years ago

No, I don't want to think bad things about MDG, simply they wanted to guarantee a fullstack experience and are trying to do a smooth migration path, but now the ES2015 (and beyond) community is too strong and mantaining the Meteor platform "as is" is just slowing down the adoption of what is cool outside of Meteor (DDP).

Interesting Matt Debergalis (MDG) interview https://www.youtube.com/watch?v=hA4sToBHkPs

AdamBrodzinski commented 9 years ago

Yea i think that their solution was great 4 years ago when it first came out and AMD was the only browser module system and Browserify was buggy at best. However now that NPM is being used for clientside JS, it makes more sense to switch.

jedwards1211 commented 9 years ago

Wow, I didn't realize Meteor was 4 years old. Though that definitely explains the package structure.

jedwards1211 commented 9 years ago

So as far as getting React from the Webpack bundle into Meteor, I just discovered that by cloning the react-runtime package into the meteor/packages folder and commenting out the lines in its package.js that actually load and export React, it won't load React even though I've added the react package to the project. I checked the Meteor compiled code as well as typing React into the browser console, and sure enough, it wasn't there. As far as I understand the local copy will always override the copy from the repository as long as it has the right version number, meaning we have a viable option for supporting any Meteor packages that depend on the react package.

There would still be some more work to make the React instance used in the Webpack bundles available to them though. The obstacle is that the client bundle is currently the very last thing that gets loaded, which means its React instance wouldn't be available for any Meteor packages when they're loaded.

The solution, I assume, would be:

AdamBrodzinski commented 9 years ago

Hmm interesting! I haven't had time to dive into chunks too much... I tried reading the docs and my eyes were glazing over without any concrete examples :laughing:

That plan sounds good to me. The only thing we'd want to double check is that if we fork the react package, all the other packages have to think it's the real react package, otherwise a dupe will be loaded. I think if it's named react and in the local packages it will use the local one first but i'm not sure.

I mean at the end of the day package authors _should_ be using weak dependancies for React so in my opinion it would be the package maintainer's fault.

If the above fork didn't work, if we could just figure out how to get the chunk to load before the packages and then as long as the packages use a weak dep, no copies of React will load. Then we don't even have to maintain a fork. If this would work a script could run to grep for react or reactjs:react to check for double copies every time you boot up.

so something like this?

react
meteor.stuff.js (including JSX package for packages to consume)
webpack.bundle.js
jedwards1211 commented 9 years ago

It's the react-runtime package specifically that I'm forking, not react itself. I got it working, but I want to clean it up a bit before I push.

Heck, we could have the npm scripts insert the latest version of the real react-runtime package into our fork every time they get run!

AdamBrodzinski commented 9 years ago

Oh cool! :beers:

Should I hold off on pushing my PR with the different structure? I can have that ready tonight or I can push it later and update it if that helps.

jedwards1211 commented 9 years ago

I'm trying to think. I put the version that loads React from the Commons Chunk in a react-commons branch because it's a bit squirrely and I still consider it experimental. Other than that, I figured out a few improvements to the run scripts while working on that (waiting for the webpack --watch bundles to be output before running meteor) that I want to port to the master branch. Should have that done in a moment though

jedwards1211 commented 9 years ago

Okay, done with that. Yeah merge the master branch into your fork and then make a PR. I'll probably have to merge your changes into the react-commons branch manually.

jedwards1211 commented 9 years ago

er...I'm not thinking, the changes from the master branch won't merge cleanly into your fork anyway. Would it be too much trouble to just make sure your fork has the latest versions of the webpack config files and run scripts, adapt the paths in them as necessary, and make sure the scripts all work?

jedwards1211 commented 9 years ago

I've often wondered if there's a Webpack option to make it expose its require() so that you can use it in a REPL or browser console. I'll have to look into that

AdamBrodzinski commented 9 years ago

Would it be too much trouble to just make sure your fork has the latest versions of the webpack config files and run scripts, adapt the paths in them as necessary, and make sure the scripts all work?

Sounds good to me. I was also going to go over every commit to make sure I didn't miss anything too, so it should be up to date with master once I go over it. Then i'll add the karma stuff after that.

jedwards1211 commented 9 years ago

Aha, this may be solution for the REPL! from http://webpack.github.io/docs/configuration.html:

target

AdamBrodzinski commented 9 years ago

Wooooo nice!

jedwards1211 commented 9 years ago

Boo, I tried that but it crashes saying require is not defined. I'm guessing Meteor messes with things such that require isn't available. Do you know?

AdamBrodzinski commented 9 years ago

Hmm not sure offhand. I know that require is undefined at runtime. This will def change in the future when they have es6 modules. I think they do this because there's no package.json to manage anything required.

jedwards1211 commented 9 years ago

Yeah, plus I think Meteor tries as hard as it can to make client and server code identical...and actually I've been wondering if Node was around when Meteor started. So it's been around since 2009, but npm was only released in 2011.

jedwards1211 commented 9 years ago

gahhh whoops :)

grigio commented 9 years ago

I've hit also this issue https://github.com/meteor/meteor/issues/3666

AdamBrodzinski commented 9 years ago

@grigio what version of Node do you have installed globally? It built for me when using meteor deploy. I didn't get a chance to a raw build because I need to upgrade to 10.36 to run it. I need to install NVM this weekend too... should make things easier.

grigio commented 9 years ago

I've v0.12.4, but the issue fortunatly isn't related to this project.

I'm using rrule and the official docs say to import the lib as:

var RRule = require('rrule').RRule;

But if you do it, it doesn't fail immediatly but RRule is undefined, the correct way to import here is:

var RRule = require('rrule');
// or
import RRule from "rrule";
AdamBrodzinski commented 9 years ago

@jedwards1211 I solved the flow-router problem. You have to call FlowRouter.wait(); in client/lib first and then call FlowRouter.initialize() at the end of main_client.js. This delays the initialization of FlowRouter until after webpack is ready.

Would you care if I opened up a Wiki and added a page for Flow-Router? That would help keep the readme clean yet would still help people starting out.

jedwards1211 commented 9 years ago

Sure! Is that something I have to give you permission to do? I've never used GitHub wikis

AdamBrodzinski commented 9 years ago

Oh yea they're public I guess... just added some placeholder TODO items. I'll fill out some of these with the info from various forum/issue content. This should help make the readme a bit easier since we can just point to a FlowRouter link or Meteor Packages, etc...

tomitrescak commented 9 years ago

Fantastic work. This looks very exciting! Would you guys consider also to prepare a small example with "standard" meteor setup for those "old school" meteorite that use Blaze + FlowRouter? Just a tiny application with two routes and content rendered from Blaze templates? I'd start porting to your rig right away and be a happy tester ;)

AdamBrodzinski commented 9 years ago

Would you guys consider also to prepare a small example with "standard" meteor setup for those "old school" meteorite that use Blaze + FlowRouter?

@tomitrescak at the very least I want to make a branch on my fork. I have yet to try the spacebars loader but it seems like it should work.

I might even get some time tonight :smile: I'm currently working on a way to symlink velocity tests from the ./test folder (currently they have to go in meteor_core).

AdamBrodzinski commented 9 years ago

@tomitrescak just pushed an example: https://github.com/AdamBrodzinski/meteor-webpack-react/tree/blaze-example

The require syntax for the templates is a little weird. I'm going to open a PR with the maintainer of the spacebars loader to see whats up... you have to use .template to get to the actual template which is annoying... eg var AppPage = require('./App.html').template. You can also access them globally as usual.

Another note is that if something doesn't quite work in webpack you can always leave it in the meteor_core folder like you do today and it will still work (it will just load before the webpack code)

Let me know what you think!

tomitrescak commented 9 years ago

Adam, great news ;) I feel like your solution can really streamline my projects, but I really do not feel like rewriting everything to React since we allready have several quite large university applications running on Meteor with the Blaze + FR setup. Thanks!

EDIT: HA! You're the best ;)

tomitrescak commented 9 years ago

Works great! Going tot try to port one of the smaller applications of ours to your setup. I have a first question here and that would be ... how does this work with meteor packages? I did not see it in your video, only NPM integration. Do I just add the meteor package using meteor add ... in the meteor_core directory? and then I do not care about imports and use it in the "/app" code?

tomitrescak commented 9 years ago

Ah, not changing anything, when I run the app for the second time (all I did was ctr+c for the first time), I receive this error and page won't render :/ Do you want me to open this in your fork?

There is no route for the path: /Router._notfoundRoute @ kadira_flow-router.js?f961d732ed2b89a53d490af5979b899800aa1a5d:1937(anonymous function) @ kadira_flow-router.js?f961d732ed2b89a53d490af5979b899800aa1a5d:21113.Route.middleware @ kadira_flow-router.js?f961d732ed2b89a53d490af5979b899800aa1a5d:608nextEnter @ kadira_flow-router.js?f961d732ed2b89a53d490af5979b899800aa1a5d:4403.page.dispatch @ kadira_flow-router.js?f961d732ed2b89a53d490af5979b899800aa1a5d:4463.page.replace @ kadira_flow-router.js?f961d732ed2b89a53d490af5979b899800aa1a5d:408Router.initialize._.each.self._page.(anonymous function) @ kadira_flow-router.js?f961d732ed2b89a53d490af5979b899800aa1a5d:19723.page.start @ kadira_flow-router.js?f961d732ed2b89a53d490af5979b899800aa1a5d:304page @ kadira_flow-router.js?f961d732ed2b89a53d490af5979b899800aa1a5d:238Router.initialize @ kadira_flow-router.js?f961d732ed2b89a53d490af5979b899800aa1a5d:1980(anonymous function) @ kadira_flow-router.js?f961d732ed2b89a53d490af5979b899800aa1a5d:2407runStartupCallbacks @ startup_client.js:30ready @ startup_client.js:32
only-dev-server.js?1a90:74 [HMR] Waiting for update signal from WDS...
kadira_flow-router.js?f961d732ed2b89a53d490af5979b899800aa1a5d:1949 Uncaught Error: FlowRouter is already initializedRouter.initialize @ kadira_flow-router.js?f961d732ed2b89a53d490af5979b899800aa1a5d:1949(anonymous function) @ router.js?2784:15(anonymous function) @ client.bundle.js:1299__webpack_require__ @ client.bundle.js:521fn @ client.bundle.js:76(anonymous function) @ main_client.js?2ae9:4(anonymous function) @ client.bundle.js:1263__webpack_require__ @ client.bundle.js:521fn @ client.bundle.js:76(anonymous function) @ client.bundle.js:558__webpack_require__ @ client.bundle.js:521(anonymous function) @ client.bundle.js:544(anonymous function) @ client.bundle.js:547
client?9d1a:15 [WDS] Hot Module Replacement enabled.
App.js?8f97:7 Hello from Meteor method!

Page renders for about 0.5 second and then disappears.

EDIT: FlowRouter has FlowRouter._askedToWait flag which solves the problem, but I'm not sure where can I set it, as Flowrouter loads before everything I tried.

AdamBrodzinski commented 9 years ago

Yep packages work the same. You can do that or use the ./met script to do ./met add foo:bar

These will be loaded before webpack so it's safe to assume they'll be available globally like normal

Ah bummer, can you confirm if the meteor_core/client/lib/config is still there? I had an issue with it getting deleted before and perhaps my fork was a commit behind.

It sounds like the router is starting too early.... Basically in a lib folder FlowRouter.wait needs to be called (I think it's wait, on my phone now lol)

I'll look at thiamin the morning too.

tomitrescak commented 9 years ago

Adam, that was the problem. Adding the config file solved it. I'm just starting to wrap my head around how your rig works and it's quite awesome. Will play with it a bit more now. Thanks!

AdamBrodzinski commented 9 years ago

Cool! Yea that should be fixed in the master of the upstream... Before it deleted the entire client folder instead of just the file.

AdamBrodzinski commented 9 years ago

@tomitrescak Ah I see the issue now. Commit 39d4d0 adds a gitignore with a glob which made the lib/configs.js file not get checked in.

It just had the following in it:

// Tell FR to wait until our ES6 code is fully initialized
FlowRouter.wait();

This is now updated here

Thanks!

tomitrescak commented 9 years ago

Been playing with it 14 hours straight ;) But finally got it where I want it, including typescript support (not through loader though as I prefer to compile it in atom). FYI, for typescript all you have to do it to rename jsx to tsx and use following notation:

import * as React from "react";

interface Props {
  foo?: string;
}

interface State {
  bar: string;
}

// React.Component<Props,State>
class MyComponent extends React.Component<Props, State> {
    render() {
        return <span>{this.props.foo}</span>
    }
}

export default MyComponent;

Also to support Semantic UI or Bootstrap a file-loader had to be added with a following configuration

{
  test: /(\.png|\.gif|\.eot|\.svg|\.woff|\.ttf)/, // we need this to load fonts
  loader: 'file'
}

But that's just info for other noobies like me ;)

If you'll ever need I can prepare a typescript supported version for you (with typings).