goatslacker / alt

Isomorphic flux implementation
http://alt.js.org/
3.45k stars 322 forks source link

Error when importing AltContainer via Babel and Webpack #259

Closed neo-headz closed 9 years ago

neo-headz commented 9 years ago

Hey guys! I've been using Alt since version 0.15.4 without any problems. I recently upgraded to 0.16.5 with the intention of using withAltContext.js (on the top level component) along with AltContainer (on any child component that needs a store). Currently, I'm using Iso, Alt instances, and react-router. I'm injecting the Alt instance as props via react-router like so:

iso.bootstrap(function clientRender (state, _, container) {
  alt.bootstrap(state);
  Router.run(routes, Router.HistoryLocation, function handleRoute (Handler) {
    var component = React.createElement(Handler, alt);
    React.render(component, container);
  });
});

The problem I'm having is whenever I try to simply import the AltContainer into any component import AltContainer from 'alt/AltContainer'; I get this error from the server and client bundle. Error: Cannot find module 'es-symbol'

Maybe this is not a problem with Alt but a problem with Webpack not resolving the dependencies correctly? I noticed that the es-symbol require call lives in alt/mixins/Subscribe.js

I'm fairly new to Node, Webpack, React, and Alt so I don't quite know how to go about debugging this. Out of curiosity, I installed es6-symbol in my project but then I get a different error saying that my stores are undefined. (yes I've already added all stores and actions within the Alt instance)

Any thoughts?

goatslacker commented 9 years ago

Sounds like a bug. I'll have a look

goatslacker commented 9 years ago

Wait how did you install this? npm install alt?

It's strange that es-symbol is not being found.

es6-symbol is not the same as es-symbol FYI they're different packages.

Can you post a full stack trace?

neo-headz commented 9 years ago

Yep, using npm install alt. I've tried removing and reinstalling just the alt package. I've also removed node_modules and reinstalled everything. Looks like npm is pulling in the correct dependencies upon install.

alt@0.16.5 node_modules/alt
├── es-symbol@1.1.2
├── flux@2.0.3
└── eventemitter3@0.1.6

I noticed the difference between es6-symbol and es-symbol, my-bad. Thanks. Here's the stack trace

Error: Cannot find module 'es-symbol'
    at Function.Module._resolveFilename (module.js:336:15)
    at Function.Module._load (module.js:278:25)
    at Module.require (module.js:365:17)
    at require (module.js:384:17)
    at Object.<anonymous> (/Users/jherrera/Legacy/front-end/build/server.js:34072:55)
    at __webpack_require__ (/Users/jherrera/Legacy/front-end/build/server.js:18:53)
    at Object.<anonymous> (/Users/jherrera/Legacy/front-end/build/server.js:16834:24)
    at __webpack_require__ (/Users/jherrera/Legacy/front-end/build/server.js:18:53)
    at Object.<anonymous> (/Users/jherrera/Legacy/front-end/build/server.js:16810:27)

I'm using node 0.12.0, npm 2.5.1, and webpack 1.8.4

goatslacker commented 9 years ago

This may be an issue with webpack, can you post your webpack config?

It looks like this its failing on mixins/Subscribe.js

neo-headz commented 9 years ago

Here's my webpack config

var _ = require('lodash');
var webpack = require('webpack');
var argv = require('minimist')(process.argv.slice(2));
var path = require('path');

var DEBUG = !argv.release;

var AUTOPREFIXER_LOADER = 'autoprefixer-loader?{browsers:' + JSON.stringify([
    'Android 2.3',
    'Android >= 4',
    'Chrome >= 20',
    'Firefox >= 24',
    'Explorer >= 8',
    'iOS >= 6',
    'Opera >= 12',
    'Safari >= 6']) + '}';

var GLOBALS = {
  'process.env.NODE_ENV': DEBUG ? '"development"' : '"production"',
  '__DEV__': DEBUG
};

//
// Common configuration chunk to be used for both
// client-side (app.js) and server-side (server.js) bundles
// -----------------------------------------------------------------------------

var config = {
  output: {
    path: './build/',
    publicPath: './',
    sourcePrefix: '  '
  },

  cache: DEBUG,
  debug: DEBUG,
  devtool: DEBUG ? '#inline-source-map' : false,

  stats: {
    colors: true,
    reasons: DEBUG
  },

  plugins: [
    new webpack.optimize.OccurenceOrderPlugin()
  ],

  resolve: {
    alias: {__SRC__: path.resolve(__dirname, 'src') + '/'},
    extensions: ['', '.webpack.js', '.web.js', '.js', '.jsx']
  },

  module: {
    preLoaders: [
      {
        test: /\.js$/,
        exclude: /node_modules/,
        loader: 'eslint'
      }
    ],

    //TODO add stylus sourcemaps, csslint-loader, and an image optimization loader
    loaders: [
      {
        test: /\.json/,
        loader: 'json'
      },
      {
        test: /\.css$/,
        loader: 'style!css!' + AUTOPREFIXER_LOADER + '!csscomb'
      },
      {
        test: /\.styl$/,
        loader: 'style!css!stylus'
      },
      {
        test: /\.gif/,
        loader: 'url?limit=10000&mimetype=image/gif'
      },
      {
        test: /\.jpg/,
        loader: 'url?limit=10000&mimetype=image/jpg'
      },
      {
        test: /\.png/,
        loader: 'url?limit=10000&mimetype=image/png'
      },
      {
        test: /\.svg/,
        loader: 'url?limit=10000&mimetype=image/svg+xml'
      },
      {
        test: /\.jsx?$/,
        exclude: /node_modules/,
        loader: 'babel'
      }
    ]
  },
  stylus: {use: [(require('nib'))(), (require('rupture'))(), (require('jeet'))(), (require('autoprefixer-stylus'))()]}
};

//
// Configuration for the client-side bundle (app.js)
// -----------------------------------------------------------------------------

var appConfig = _.merge({}, config, {
  entry: './src/app.js',
  output: {
    filename: 'app.js'
  },
  plugins: config.plugins.concat([
      new webpack.DefinePlugin(_.merge(GLOBALS, {'__SERVER__': false}))
    ].concat(DEBUG ? [] : [
        new webpack.optimize.DedupePlugin(),
        new webpack.optimize.UglifyJsPlugin(), // NOTE: this will also minify CSS
        new webpack.optimize.AggressiveMergingPlugin()
      ])
  )
});

//
// Configuration for the server-side bundle (server.js)
// -----------------------------------------------------------------------------

var serverConfig = _.merge({}, config, {
  entry: './src/server.js',
  output: {
    filename: 'server.js',
    libraryTarget: 'commonjs2'
  },
  target: 'node',
  externals: /^[a-z][a-z\.\-0-9]*$/,
  node: {
    console: false,
    global: false,
    process: false,
    Buffer: false,
    __filename: false,
    __dirname: false
  },
  plugins: config.plugins.concat(
    new webpack.DefinePlugin(_.merge(GLOBALS, {'__SERVER__': true}))
  ),
  module: {
    loaders: config.module.loaders.map(function (loader) {
      // Remove style-loader
      return _.merge(loader, {
        loader: loader.loader = loader.loader.replace('style!', '')
      });
    })
  }
});

module.exports = [appConfig, serverConfig];
goatslacker commented 9 years ago

I just required AltContainer successfully:

alt@0.16.6, webpack@1.9.10, node@0.10.x

My app.js and server.js looks like:

/*eslint-env node*/
var Alt = require("alt");
var AltContainer = require("alt/AltContainer");
var React = require("react");

var alt = new Alt();

var node = React.createElement(AltContainer, { flux: alt });

module.exports = node;
goatslacker commented 9 years ago

I just have that + the webpack.config.js in my directory

vkbansal commented 9 years ago

I too am facing similar problem with webpack. but I'm not using AltContainer.

What I get is

> Uncaught TypeError: Cannot read property 'Symbol()' of undefined
createActions   @   index.js:140

This problem only occurs when I disable webpack.optimize.UglifyJsPlugin.

I had opened an issue on webpack too => webpack/webpack#1112

Update: I tried the same with browserify. Everything works after uglify, but throws the same error before.

goatslacker commented 9 years ago

@vkbansal which version of alt are you on?

neo-headz commented 9 years ago

@goatslacker I can reproduce your results. Thanks for that simple test. However, if you run the server.js file after it has been packed, you'll see the error. I created a small repo to easily reproduce.

That aside, I figured out the problem and it's NOT Alt. My webpack server bundle config is incorrect. The externals option is the culprit. I changed the regex into an object representing node_modules as externals. The object has a key/value of each module name but value is prepended with "commonjs" as suggested here. No more error!

var nodeModules = {};
fs.readdirSync('node_modules')
  .filter(function (x) {
    return ['.bin'].indexOf(x) === -1;
  })
  .forEach(function (mod) {
    nodeModules[mod] = 'commonjs ' + mod;
  });

externals: nodeModules,

troutowicz commented 9 years ago

Is there a particular reason you are making every dependency an external?

vkbansal commented 9 years ago

@goatslacker I'm using 0.16.x and like @headz68 I too am loading alt as external depenedency

My webpack confing

var webpack = require("webpack"),
    path = require("path");

module.exports = {
    entry: {
        "vendor": [
            "react",
            "alt",
            "immutable",
            "./src/flux.js"
        ],
        "app": "./src/app.js"
    },
    output: {
        path: path.join(__dirname, "dev"),
        filename: "[name].js",
        sourceMapFileName: "[name].js.map"
    },
    module: {
        loaders: [
            {
                test: /\.js$/,
                loaders: [/*"react-hot",*/ "babel?stage=0"],
                include: path.join(__dirname, "src")
            }
        ]
    },
    devtool: "source-map",
    external: {
        react: {
            root: "React",
            commonjs2: "react",
            commonjs: "react",
            amd: "react"
        },
        jquery: {
            root: "jQuery",
            commonjs2: "jquery",
            commonjs: "jquery",
            amd: "jquery"
        }
    },
    plugins: [
        new webpack.optimize.OccurenceOrderPlugin(),
        new webpack.DefinePlugin({
            "process.env": {
                "NODE_ENV": JSON.stringify("production")
            }
        }),
        // new webpack.optimize.UglifyJsPlugin({
        //     compressor: {
        //         warnings: false
        //     },
        //     mangle: false
        // }),
        new webpack.optimize.CommonsChunkPlugin({
            name: "vendor",
            filename: "vendor.js",
            minChunks: Infinity
        })
    ]
};
neo-headz commented 9 years ago

@troutowicz I'm still a little confused on how the externals property should be configured for server bundles. It's my understanding that when using webpack for server bundles, there is an issue with binary dependencies and webpack changing the require call into a global var because webpack assumes a browser environment. Setting all dependencies as external leaves the require call. My config was originally forked from the config in this starter kit. This is the first time I've run into and sort of dependency issues. I also noticed a similar conversation in this thread. Are you suggesting that there is an alternative/better solution?

troutowicz commented 9 years ago

Yeah, your solution does seem to be the best way to externalize all dependencies. But webpack will export your bundle in whatever format you tell it to use, that is what output.libraryTarget is for. In my project, the only external I have is React, and I use AltContainer just fine. Also, webpack compiles for whatever environment you tell it to use, browser is default, but target can be set to many different environments.

neo-headz commented 9 years ago

@troutowicz hmm, I have libraryTarget: 'commonjs2' and target: 'node' as you can see in my original config above but without setting all node_modules as externals I still get the error.

jackcallister commented 9 years ago

@headz68 Try changing your externals to this regex /^[^.].*$/ - It looks like webpack is bundling things with a slash like react/addons and alt/container.

jackcallister commented 9 years ago

@headz68 Scratch that, the regex will bork when using @import statements (with less). I've switched to specifying react and alt (you'll need react/addons in there to prevent react-router loosing context).

externals: [
    {
      'alt/container': true,
      'react/addons': true
    },
    /^[a-z\-0-9]+$/
 ],
goatslacker commented 9 years ago

Closing this but we can keep discussing webpack.

joshhornby commented 9 years ago

@jarsbe Tried this solution and not having any luck. Is this still valid code?

jackcallister commented 9 years ago

@joshhornby In my case (alt 0.16.5) yes. You may have another dependency which the externals regex is not matching. Have a look in your bundled JS to find the culprit.

joshhornby commented 9 years ago

@jarsbe Any clues as to what I am looking for?

jackcallister commented 9 years ago

If I remember correctly the server side bundle was including react and alt as code (rather than require('mylib')) - you are looking for libraries that have been bundled which shouldn't be. Do you have a stack trace or code I can look at?

joshhornby commented 9 years ago

Sure - this is from the terminal.

module.js:338
    throw err;
    ^
Error: Cannot find module 'es-symbol'
    at Function.Module._resolveFilename (module.js:336:15)
    at Function.Module._load (module.js:278:25)
    at Module.require (module.js:365:17)
    at require (module.js:384:17)
    at Object.<anonymous> (/Users/joshuahornby/code/my-site/public/assets/app.server.js:57131:19)
    at __webpack_require__ (/Users/joshuahornby/code/my-site/public/assets/app.server.js:21:30)
    at Object.<anonymous> (/Users/joshuahornby/code/my-site/public/assets/app.server.js:57099:15)
    at __webpack_require__ (/Users/joshuahornby/code/my-site/public/assets/app.server.js:21:30)
    at Object.<anonymous> (/Users/joshuahornby/code/my-site/public/assets/app.server.js:56916:18)
    at __webpack_require__ (/Users/joshuahornby/code/my-site/public/assets/app.server.js:21:30)
    at Object.<anonymous> (/Users/joshuahornby/code/my-site/public/assets/app.server.js:56898:23)
    at __webpack_require__ (/Users/joshuahornby/code/my-site/public/assets/app.server.js:21:30)
    at Object.<anonymous> (/Users/joshuahornby/code/my-site/public/assets/app.server.js:53245:35)
    at Object.<anonymous> (/Users/joshuahornby/code/my-site/public/assets/app.server.js:53386:28)
    at __webpack_require__ (/Users/joshuahornby/code/my-site/public/assets/app.server.js:21:30)
    at Object.<anonymous> (/Users/joshuahornby/code/my-site/public/assets/app.server.js:2341:37)
jackcallister commented 9 years ago

@joshhornby Ah install es-symbol explicitly npm install es-symbol --save. Didn't dig into why but that fixed it for me.

goatslacker commented 9 years ago

0.16.5 should have es-symbol included with alt