Webpack everything #626

mjackson · 2014-12-18T19:16:08Z

This branch is an attempt to use webpack to build our tests instead of Browserify.

I'm currently having an issue that I hope @sokra can help with in this branch. When I run ./scripts/test I get the following error:

ERROR [karma]: [TypeError: Not a string or buffer]
TypeError: Not a string or buffer
    at Hash.update (crypto.js:209:17)
    at sha1 (/Users/michael/Projects/react-router/node_modules/karma-webpack/index.js:129:7)
    at /Users/michael/Projects/react-router/node_modules/karma-webpack/index.js:151:15
    at MemoryFileSystem.(anonymous function) [as readFile] (/Users/michael/Projects/react-router/node_modules/webpack-dev-server/node_modules/webpack-dev-middleware/node_modules/memory-fs/lib/MemoryFileSystem.js:205:11)
    at doRead (/Users/michael/Projects/react-router/node_modules/karma-webpack/index.js:117:32)
    at Plugin.readFile (/Users/michael/Projects/react-router/node_modules/karma-webpack/index.js:120:3)
    at process._tickCallback (node.js:419:13)

Digging a bit deeper, it looks like the memory-fs module is having problems finding some of the files it needs. The two errors I'm seeing are:

/_js/modules/__tests__/Router-test.js Error: Path doesn't exists /_js/modules/__tests__/Router-test.js
/_js/modules/components/__tests__/DefaultRoute-test.js Error: Path doesn't exists /_js/modules/components/__tests__/DefaultRoute-test.js

@sokra Would you mind taking a look at this branch and telling me what I'm doing wrong?

mjackson · 2014-12-18T21:56:35Z

@sokra I'm digging a bit further, basically just putting a bunch of debug statements in the memory-fs module to see what it's doing, and I'm getting the following:

mkdirpSync /
mkdirpSync /_js/modules/locations/__tests__
writeFileSync /_js/modules/locations/__tests__/HashLocation-test.js undefined
mkdirpSync /_js/modules/utils/__tests__
writeFileSync /_js/modules/utils/__tests__/Path-test.js undefined
readFileSync /_js/modules/__tests__/Router-test.js undefined
ERROR [karma]: [TypeError: Not a string or buffer]
TypeError: Not a string or buffer
    at Hash.update (crypto.js:209:17)
    at sha1 (/Users/michael/Projects/react-router/node_modules/karma-webpack/index.js:129:7)
    at /Users/michael/Projects/react-router/node_modules/karma-webpack/index.js:151:15
    at MemoryFileSystem.readFile (/Users/michael/Projects/react-router/node_modules/webpack-dev-server/node_modules/webpack-dev-middleware/node_modules/memory-fs/lib/MemoryFileSystem.js:206:10)
    at doRead (/Users/michael/Projects/react-router/node_modules/karma-webpack/index.js:117:32)
    at Plugin.readFile (/Users/michael/Projects/react-router/node_modules/karma-webpack/index.js:120:3)
    at process._tickCallback (node.js:419:13)
readFileSync /_js/modules/components/__tests__/DefaultRoute-test.js undefined
ERROR [karma]: [TypeError: Not a string or buffer]
TypeError: Not a string or buffer
    at Hash.update (crypto.js:209:17)
    at sha1 (/Users/michael/Projects/react-router/node_modules/karma-webpack/index.js:129:7)
    at /Users/michael/Projects/react-router/node_modules/karma-webpack/index.js:151:15
    at MemoryFileSystem.readFile (/Users/michael/Projects/react-router/node_modules/webpack-dev-server/node_modules/webpack-dev-middleware/node_modules/memory-fs/lib/MemoryFileSystem.js:206:10)
    at doRead (/Users/michael/Projects/react-router/node_modules/karma-webpack/index.js:117:32)
    at Plugin.readFile (/Users/michael/Projects/react-router/node_modules/karma-webpack/index.js:120:3)
    at process._tickCallback (node.js:419:13)

It looks like karma-webpack is asking memory-fs to readFileSync('/_js/modules/__tests__/Router-test.js') before it's ever written. The strange thing is, this feels like a race condition somewhere in karma-webpack because every once in a while I'll get a different filename that is missing.

Closes #625 Closes #608

gaearon · 2014-12-19T01:31:53Z

You still need any help on this?

ryanflorence · 2014-12-19T01:56:58Z

This looks awesome. I've been wanting to do it but haven't made the time for it.

I've been wondering if we ought to run the test suite against the global build too, now might be a good time to do that? Or I guess we can do a commit after this.

ryanflorence · 2014-12-19T02:00:06Z

package.json

I've always preferred the other way, have the npm run scripts simply call a scripts/something so that you don't do too much in here (and so the scripts are more portable), but anyway ... I don't care enough to say "no" to this commit.

I've been leaning more and more towards putting as much in my package.json as I can. It's more portable, and keeps a bunch of extra files from popping up in my repos.

ryanflorence · 2014-12-19T02:04:31Z

Uh ... this creates a 62kb gzipped build, on master with browserify its only 13kb.

👎

gaearon · 2014-12-19T02:25:19Z

~~I don't know if it makes a difference but I thought DefinePlugin should be used this way:~~

        new webpack.DefinePlugin({
          'process.env': {
            'NODE_ENV': whatever
          }
        }),

~~and not 'process.env.NODE_ENV': whatever~~

seems like it doesn't: envifying actually worked

gaearon · 2014-12-19T02:28:28Z

Also, there's a whole React in there..

gaearon · 2014-12-19T02:32:03Z

I think you forgot this in Webpack config:

  externals: {
    'react': 'React'
  },

ryanflorence · 2014-12-19T02:34:58Z

that looks right http://webpack.github.io/docs/library-and-externals.html

ryanflorence · 2014-12-19T02:36:12Z

That drops it down to 18kb gzipped.

5kb gzipped doesn't seem like its worth using webpack for the global build.

gaearon · 2014-12-19T02:40:41Z

How do you check length? gzip -c react-router.js | wc -c gives me 15521..

ryanflorence · 2014-12-19T02:41:01Z

when you run scripts/build it tells you already

gaearon · 2014-12-19T02:42:11Z

Indeed, using Webpack's Uglify plugin gives better results than having it as a separate step.

ryanflorence · 2014-12-19T02:42:25Z

oh are we double minifying?

gaearon · 2014-12-19T02:43:03Z

Not double minifying, but for some reason, letting Webpack uglify itself (by using its plugin) works better than uglifying later.

ryanflorence · 2014-12-19T02:43:31Z

still 2kb bigger than the browserify build ... hmmmmmmm

gaearon · 2014-12-19T02:47:15Z

I think I got it to 10619, wonder if it's working..

gaearon · 2014-12-19T02:48:57Z

gzipped, the global build is:
   10619 bytes

My config:

var webpack = require('webpack');

module.exports = {
  // don't include Node Buffer polyfill (why is it there by default?)
  // http://webpack.github.io/docs/configuration.html#node
  node: {
    buffer: false
  },

  output: {
    library: 'ReactRouter',
    libraryTarget: 'var'
  },

  externals: {
    'react': 'React'
  },

  plugins: [
    new webpack.DefinePlugin({
      'process.env.NODE_ENV': JSON.stringify(process.env.NODE_ENV || 'production')
    }),

    // let Webpack uglify
    new webpack.optimize.UglifyJsPlugin({
       compressor: {
         warnings: false
       }
    })
  ]

};

This is for the minimized file, you don't need to uglify after that.

Note Webpack by default include Buffer polyfill, I excluded that.

I haven't checked if this works.

mjackson · 2014-12-19T04:19:55Z

@gaearon Good call. Using webpack's uglify plugin gets the global build down to 10k

@rpflorence saved 3k!

ryanflorence · 2014-12-19T07:04:01Z

Awesome, hopefully it still works.

I'd like to have a non-minified version too, still. We've got apps that need the global build but we don't want it minified in dev.

gaearon · 2014-12-19T11:08:40Z

Yeah, you just need to push or not push that plugin based on a condition (env?)

mjackson · 2014-12-19T15:37:35Z

@rpflorence Updated.

Everything should still work fine. We don't rely on the Buffer module for anything, and we're not doing anything fancy with uglify.

Webpack everything

sokra · 2014-12-19T20:46:48Z

👍

gaearon · 2014-12-19T20:49:29Z

@sokra While you’re here, curious: why is Buffer in by default?

On 19 Dec 2014, at 21:46 , Tobias Koppers notifications@github.com wrote:

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

sokra · 2014-12-19T20:52:26Z

Sorry I'm on vacation and cannot investigate in detail, but you can see it in the analyse tool or with --display-reasons.

gaearon · 2014-12-19T20:57:49Z

Ah, have a good rest!

sokra · 2014-12-22T14:06:53Z

Buffer is used by your version of qs:

// qs/lib/utils.js
exports.isBuffer = function (obj) {

    if (typeof Buffer !== 'undefined') {
        return Buffer.isBuffer(obj);
    }
    else {
        return false;
    }
};

The newer version solved it in another way. You just need to upgrade to qs@2.3.3.

mjackson · 2014-12-22T15:41:09Z

Done! Thanks @sokra

Michael Jackson
@mjackson

On Mon, Dec 22, 2014 at 6:06 AM, Tobias Koppers notifications@github.com
wrote:

Buffer is used by your version of qs:

// qs/lib/utils.jsexports.isBuffer = function (obj) {
if (typeof Buffer !== 'undefined') {
    return Buffer.isBuffer(obj);
}
else {
    return false;
}
};

The newer version solved it in another way. You just need to upgrade to
qs@2.3.3.

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

mjackson added 2 commits December 18, 2014 09:59

[changed] Run the examples with npm run examples

94dc231

Use karma's built-in globbing

b9dbce7

mjackson mentioned this pull request Dec 18, 2014

Promote envify to dependencies and add browserify transform config for it #625

Closed

mjackson force-pushed the webpack-everything branch from 2fb8f1a to f041c2a Compare December 18, 2014 21:18

mjackson changed the title ~~Webpack everything - DO NOT MERGE~~ Webpack tests - DO NOT MERGE Dec 18, 2014

mjackson mentioned this pull request Dec 18, 2014

how to get source maps? codymikol/karma-webpack#12

Closed

mjackson force-pushed the webpack-everything branch from f041c2a to 144e8cc Compare December 19, 2014 00:21

mjackson changed the title ~~Webpack tests - DO NOT MERGE~~ Webpack everything Dec 19, 2014

mjackson force-pushed the webpack-everything branch from 690fce6 to 7a0c786 Compare December 19, 2014 00:26

mjackson added 2 commits December 18, 2014 17:12

[changed] Use webpack instead of Browserify

b98f5a1

Closes #625 Closes #608

Fix JSHint warnings

4443baf

mjackson force-pushed the webpack-everything branch from 9742434 to 4443baf Compare December 19, 2014 01:12

ryanflorence reviewed Dec 19, 2014
View reviewed changes

add react as external

6b8e4e0

Use webpack's uglify plugin

ffe3e46

mjackson force-pushed the webpack-everything branch from c543305 to ffe3e46 Compare December 19, 2014 15:34

mjackson added a commit that referenced this pull request Dec 19, 2014

Merge pull request #626 from rackt/webpack-everything

f1e68fb

Webpack everything

mjackson merged commit f1e68fb into master Dec 19, 2014

mjackson deleted the webpack-everything branch February 12, 2015 23:57

lock bot locked as resolved and limited conversation to collaborators Jan 21, 2019

Uh oh!

Webpack everything #626

Webpack everything #626

Uh oh!

Conversation

mjackson commented Dec 18, 2014

Uh oh!

mjackson commented Dec 18, 2014

Uh oh!

gaearon commented Dec 19, 2014

Uh oh!

ryanflorence commented Dec 19, 2014

Uh oh!

ryanflorence Dec 19, 2014

Choose a reason for hiding this comment

Uh oh!

mjackson Dec 19, 2014

Choose a reason for hiding this comment

Uh oh!

ryanflorence commented Dec 19, 2014

Uh oh!

gaearon commented Dec 19, 2014

Uh oh!

gaearon commented Dec 19, 2014

Uh oh!

gaearon commented Dec 19, 2014

Uh oh!

ryanflorence commented Dec 19, 2014

Uh oh!

ryanflorence commented Dec 19, 2014

Uh oh!

gaearon commented Dec 19, 2014

Uh oh!

ryanflorence commented Dec 19, 2014

Uh oh!

gaearon commented Dec 19, 2014

Uh oh!

ryanflorence commented Dec 19, 2014

Uh oh!

gaearon commented Dec 19, 2014

Uh oh!

ryanflorence commented Dec 19, 2014

Uh oh!

gaearon commented Dec 19, 2014

Uh oh!

gaearon commented Dec 19, 2014

Uh oh!

mjackson commented Dec 19, 2014

Uh oh!

ryanflorence commented Dec 19, 2014

Uh oh!

gaearon commented Dec 19, 2014

Uh oh!

mjackson commented Dec 19, 2014

Uh oh!

sokra commented Dec 19, 2014

Uh oh!

gaearon commented Dec 19, 2014

Uh oh!

sokra commented Dec 19, 2014

Uh oh!

gaearon commented Dec 19, 2014

Uh oh!

sokra commented Dec 22, 2014

Uh oh!

mjackson commented Dec 22, 2014

Uh oh!

Uh oh!