survivejs / webpack-book

From apprentice to master (CC BY-NC-ND)
https://survivejs.com/webpack/
2.42k stars 319 forks source link

Optimizing-rebundling.md not working with new react-dom package #35

Closed romseguy closed 7 years ago

romseguy commented 9 years ago

ERROR in ./~/react-dom/index.js [0] Module not found: Error: Cannot resolve 'file' or 'directory' /home/romseguy/myproject/node_modules/react/dist/react.min.js/lib/ReactDOMClient in /home/romseguy/myproject/discount-ascii-warehouse-0.2.0/node_modules/react-dom

https://christianalfoni.github.io/react-webpack-cookbook/Optimizing-rebundling.html

Is there a way to prevent using the alias over the node_modules directory?

bebraw commented 9 years ago

Yeah, I can see the split can be problematic. Perhaps we need to go through loaders instead? In this case you would resolve against React at the test and use imports-loader to inject react-dom. You can likely set up an alias for react-dom too in order to use the minified version for that as well.

I haven't tried this approach yet but it would be the first thing for me to try. If you get it work this way, let me know. :+1:

NickStefan commented 9 years ago

im also hitting this problem. is there a definite solution yet?

bebraw commented 9 years ago

@NickStefan I might try setting up some separate task to generate a minified version based on the bits you need (minify in development mode to retain type checks!). The minified bundle would then contain React core and react-dom. Of course this implies extra work but it might be a direction worth investigating.

NickStefan commented 9 years ago

@bebraw thanks for the quick answer! I set up a separate task, and added aliases for react and reactDom

var pathToReactMin = path.resolve(node_modules, 'react/dist/react.min.js');
var pathToReactDomMin = path.resolve(node_modules, 'react/lib/ReactDOM');

works. thanks.

michaelahlers commented 8 years ago

:+1:

NickStefan commented 8 years ago

React has a 'release' version for react-dom as well:

var pathToReactMin = path.resolve(node_modules, 'react/dist/react.min.js');
var pathToReactDomMin = path.resolve(node_modules, 'react-dom/dist/react-dom.min.js');
michaelahlers commented 8 years ago

@NickStefan, I wound up using resources already included with the React NPM distribution:

// ...

var reactPath = path.join(modulesPath, 'react', 'react.js')
  , reactDOMPath = path.join(modulesPath, 'react', 'lib', 'ReactDOM.js')
  , reactCSSTransitionGroupPath = path.join(modulesPath, 'react', 'lib', 'ReactCSSTransitionGroup.js');

// ...

module.exports = {
  // ...

  entry: {
    // ...

    vendors: [
      'react'
      , 'react-dom'
      , 'react-addons-css-transition-group'
    ]
  },

  resolve: {
    alias: {
      'react': reactPath,
      'react-dom': reactDOMPath,
      'react-addons-css-transition-group': reactCSSTransitionGroupPath
    }
  }

  // ...
}

Open to criticism.

EnchanterIO commented 8 years ago

@michaelahlers I have the same problem and was solved by your approach of 'react/lib/ReactDOM' but why official React page says to install: "npm install --save react react-dom" when later we provide ReactDOM through /react/lib/ReactDOM ? In that case I can just remove the node_modules/react-dom

michaelahlers commented 8 years ago

@TrkiSF2, because that documentation makes assumptions that don't hold true in these webpack configurations.

Look inside those NPM modules. Specifically, for this scenario, see index.js in react-dom and react-addons-css-transition-group respectively:

module.exports = require('react/lib/ReactDOM');
module.exports = require('react/lib/ReactCSSTransitionGroup');

These modules aren't providing resources—they're essentially references back to the react package contents, which I've effectively duplicated in my webpack alias configuration.

It's obviously a hack, and can't possibly be best practice.

EnchanterIO commented 8 years ago

I know, was looking there. At the beginning I wanted to try something like:

join(
    'node_modules',
    'react-dom',
    'index.js',

which would be cleaner imo but didn't work. Well for now I am keeping it the way you proposed.

NickStefan commented 8 years ago

They're referencing react-dom back to the original react repro to get you used to the package separation, until they can actually separate them out. The API changed before the underneath did. Deprecation vs outright dropping APIs. Once the packages are actually more separate, I imagine it will be less weird with the aliasing in webpack.

aabanaag commented 8 years ago

I have this webpack.config.js but I still get a Undefined: require is not defined error in react-dom.js when running npm run dev.

Is there something wrong with my setup?

var path = require('path');
var webpack = require('webpack');
var node_modules = path.resolve(__dirname, 'node_modules');
var react_path = path.resolve(node_modules, 'react/dist/react');
var react_dom_path = path.resolve(node_modules, 'react/lib/ReactDOM');

module.exports = {
   entry: [...],
   resolve: {
      alias: { 'react': react_path, 'react-dom': react_dom_path }
   },
   output: {...},
   module: {
      loaders: [...],
      noParse: [react_path, react_dom_path]
   }
};
ghost commented 8 years ago

@aabanaag I got that too. Seems like different things are in conflict. Once you alias 'react' to 'react.min.js', the underlying require('react/lib/ReactDOM') in 'react-dom' becomes relative to 'react.min.js', which breaks when building. But if you alias 'react-dom' to 'react-dom.min.js' and noParse it as usual, the require('react') in 'react-dom.min.js' won't be converted to a webpack_require call, so that breaks in the browser. @michaelahlers doesn't include noParse in his code snippet, so maybe he didn't hit that.

It seems to work if you alias 'react' and 'react-dom' to their 'dist/[name].min.js' copies, but only noParse 'react', so that the require('react') in 'react-dom.min.js' gets processed by Webpack and turned into a webpack_require call to get React.

It's fairly verbose, but something like:

var alias = {
  'react': 'react/dist/react.min.js',
  'react-dom': 'react-dom/dist/react-dom.min.js'
};
Object.keys(alias).forEach(function(name) {
  alias[name] = path.resolve(path.join(__dirname, 'node_modules', alias[name]));
});

var noParse = ['react'].map(name => alias[name]);

All 'react-dom.min.js' does it require React and return its __SECRET_DOM_DO_NOT_USE_OR_YOU_WILL_BE_FIRED property, which is a reference to ReactDOM. It's a really small file, so taking it out of noParse shouldn't really matter.

aabanaag commented 8 years ago

@chrispesto But how about using production version and not the minified one. Does it apply too?

ghost commented 8 years ago

@aabanaag You mean 'react-dom.js' instead of 'react-dom.min.js'? Yes, you'd still need to remove it from the noParse, because it has require('react').

erwagasore commented 8 years ago

You can expose React as a global variable using expose-loader (npm i expose-loader --save-dev)

module = {
    loaders: [{
      test: path_to_your_react_min_location,
      loader: "expose?React"
    }]
}

Then based on @chrispesto approach then noParse variable would be like this var noParse = Object.keys(alias).map(name => alias[name]);

aunz commented 8 years ago

same issue here, Undefined: require is not defined error in react-dom.js

aj0strow commented 8 years ago

@chrispesto thank you! Works great for me, no extra build time really.

bebraw commented 7 years ago

I'm going to close this as the book has drifted too much apart from the initial issue. It's possible I'll add something similar there. Thanks.