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

WebPack related error: LevelUp package.json produces errors #3319

Closed
andru opened this Issue Dec 31, 2014 · 23 comments

Comments

Projects
None yet
8 participants
@andru
Copy link

andru commented Dec 31, 2014

When packaging PouchDB in an app with WebPack, the following error gets output. The current PouchDB npm release doesn't build because of a bug recently fixed (see #3287). The current github master allows it to proceed but it then hits the error below.

Unless I'm mistaken LevelUP shouldn't be required in the client? Can it be left out of the package altogether in a similar manor to the fix for #3287 ?

Is it ok to just add levelup: false to the browser section of package.json or should it be replaced with a browser compatible module?

ERROR in ./~/pouchdb/~/levelup/package.json
Module parse failed: /Volumes/Files/Projects/Harvest/app2/node_modules/pouchdb/node_modules/levelup/package.json Line 2: Unexpected token :
You may need an appropriate loader to handle this file type.
| {
|   "name": "levelup",
|   "description": "Fast & simple storage - a Node.js-style LevelDB wrapper",
|   "version": "0.18.6",
 @ ./~/pouchdb/~/levelup/lib/util.js 102:30-56
@daleharvey

This comment has been minimized.

Copy link
Member

daleharvey commented Dec 31, 2014

@svnlto has a recent commit that helped get this working in webpack, have you been trying with master or the last release?

Since we use the leveldb browser versions (leveljs etc), not sure we can straight use levelup: false but it may be that we only need to switch leveldown in browserify and turning levelup off will be fine (I dont know a bunch about webpack)

@svnlto

This comment has been minimized.

Copy link
Contributor

svnlto commented Dec 31, 2014

Here's what needs to be done (in webpack's config), to get it working with pouchdb:

  module: {
    loaders: [{
      test: /\.json$/,
      loader: 'json'
    },
    {
      test: /\.j(s|sx)$/,
      loader: 'react-hot!es6-loader!jsx-loader?harmony',
      exclude: /node_modules/
    }],
    noParse: /lie.js/
  },

also, you should work from master, it contains the additional fix.

not sure we can straight use levelup: false but it may be that we only need to switch leveldown in browserify and turning levelup off will be fine

perhaps that might be something we'd want to look at as well.

@andru

This comment has been minimized.

Copy link

andru commented Dec 31, 2014

Awesome! I was on master already, but that config has the levelup problem fixed. Thanks @svnlto.

@svnlto

This comment has been minimized.

Copy link
Contributor

svnlto commented Feb 27, 2015

@daleharvey I've picked this up again with more recent versions of webpack and pouchdb. I've had to make some addition to PouchDB's package.json file to get it working this time round:

"browser": {
  "levelup": false,
  "leveldown": false
}
@svnlto

This comment has been minimized.

Copy link
Contributor

svnlto commented Feb 28, 2015

I had to remove noParse. So now I'm facing another issue. Not quite sure if this is the right place tho..

WARNING in ./~/pouchdb/~/lie/~/immediate/dist/immediate.js
Critical dependencies:
1:440-447 This seem to be a pre-built javascript file. Even while this is possible, it's not recommended. Try to require to orginal source to get better results.
 @ ./~/pouchdb/~/lie/~/immediate/dist/immediate.js 1:440-447

WARNING in ./~/pouchdb-authentication/~/lie/~/immediate/dist/immediate.js
Critical dependencies:
1:440-447 This seem to be a pre-built javascript file. Even while this is possible, it's not recommended. Try to require to orginal source to get better results.
 @ ./~/pouchdb-authentication/~/lie/~/immediate/dist/immediate.js 1:440-447

/cc @nolanlawson @calvinmetcalf

@nolanlawson

This comment has been minimized.

Copy link
Member

nolanlawson commented Feb 28, 2015

Can you provide a PR? I'm not too familiar with webpack, but we definitely want it to at least work in webpack.

As for the warnings, they don't seem too severe. :)

@timcash

This comment has been minimized.

Copy link

timcash commented Mar 14, 2015

Trying to solve this for myself now. Any more tips @svnlto ?

with

"browser": {
"levelup": false,
"leveldown": false
}

in pouchDB package.json

WARNING in ./dev/~/pouchdb/~/lie/dist/lie.js
Critical dependencies:
1:438-445 This seems to be a pre-built javascript file. Though this is possible, it's not recommended.     Try to require the original source to get better results.
 @ ./dev/~/pouchdb/~/lie/dist/lie.js 1:438-445
@svnlto

This comment has been minimized.

Copy link
Contributor

svnlto commented Mar 14, 2015

I should have a PR ready in the next couple of days. As to the warning, try the noParse config option:

...
noParse: /lie\.js$/
...
@timcash

This comment has been minimized.

Copy link

timcash commented Mar 14, 2015

great that removes the warnings, thanks!

svnlto added a commit to svnlto/pouchdb that referenced this issue Mar 14, 2015

@MattKunze

This comment has been minimized.

Copy link

MattKunze commented Apr 2, 2015

Those steps get it working for me, but it still seems to be including a ton of stuff it doesn't need.

var PouchDb = require('pouchdb/dist/pouchdb');

generates the same warning @timcash mentioned above, but shaves ~300K off the unminified bundle for me.

@MattKunze

This comment has been minimized.

Copy link

MattKunze commented Apr 2, 2015

I created a simple repo to demonstrate: https://github.com/MattKunze/pouchdb-webpack

matt@ghee ~/tmp/pouchdb-webpack (master)$ webpack
Hash: aa2663b230d833a189fb
Version: webpack 1.5.3
Time: 1091ms
           Asset    Size  Chunks             Chunk Names
  dist-bundle.js  376864       0  [emitted]  dist
normal-bundle.js  677884       1  [emitted]  normal
   [0] ./normal.js 34 {1} [built]
   [0] ./dist.js 47 {0} [built]
    + 122 hidden modules

normal.js includes PouchDB as described in this thread, dist.js requires the dist version. You can see the file size difference above.

So it seems like there is a workable solution, but it would be nice to have it work as a native require so any other shared dependencies aren't duplicated.

nolanlawson added a commit that referenced this issue May 4, 2015

(#3319) - set leveldown to false to fix webpack
Webpack does not like that `leveldown` is trying to use
`fs`. We can harmlessly set it to `false` in the package.json.

For `levelup`, the issue is that it's trying to directly load
its own package.json file, which is apparently a big contentious
issue in Webpack, and the standard solution is that app devs add their
own json loader, which is what Webpack devs themselves advocate (see
[here](webpack/webpack#378 (comment)))
and [here](http://mattdesl.svbtle.com/browserify-vs-webpack)).

We can add a note about this Webpack config fix to our own wiki,
or we can just trust that Webpack-using devs will find it themselves
after 5 minutes of googling the JSON issue (I found it in about
as much time). In any case, the bare minimum we need to do is
set `leveldown` to `false`.

nolanlawson added a commit that referenced this issue May 4, 2015

(#3319) - set leveldown to false to fix webpack
Webpack does not like that `leveldown` is trying to use
`fs`. We can harmlessly set it to `false` in the package.json.

For `levelup`, the issue is that it's trying to directly load
its own package.json file, which is apparently a big contentious
issue in Webpack, and the standard solution is that app devs add their
own json loader, which is what Webpack devs themselves advocate (see
[here](webpack/webpack#378 (comment)))
and [here](http://mattdesl.svbtle.com/browserify-vs-webpack)).

We can add a note about this Webpack config fix to our own wiki,
or we can just trust that Webpack-using devs will find it themselves
after 5 minutes of googling the JSON issue (I found it in about
as much time). In any case, the bare minimum we need to do is
set `leveldown` to `false`.

nolanlawson added a commit that referenced this issue May 4, 2015

(#3319) - set leveldown to false to fix webpack
Webpack does not like that `leveldown` is trying to use
`fs`. We can harmlessly set it to `false` in the package.json.

For `levelup`, the issue is that it's trying to directly load
its own package.json file, which is apparently a big contentious
issue in Webpack, and the standard solution is that app devs add their
own json loader, which is what Webpack devs themselves advocate (see
[here](webpack/webpack#378 (comment)))
and [here](http://mattdesl.svbtle.com/browserify-vs-webpack)).

We can add a note about this Webpack config fix to our own wiki,
or we can just trust that Webpack-using devs will find it themselves
after 5 minutes of googling the JSON issue (I found it in about
as much time). In any case, the bare minimum we need to do is
set `leveldown` to `false`.
@nolanlawson

This comment has been minimized.

Copy link
Member

nolanlawson commented May 7, 2015

So this issue is fixed; I can webpack ./lib/index.js out.js and it works fine.

For the JSON problem, the issue is that levelup is trying to directly load
its own package.json file, which is apparently a big contentious
issue in Webpack, and the standard solution is that app devs add their
own json loader, which is what Webpack devs themselves advocate (see
[here](webpack/webpack#378 %28comment%29) and here).

@nolanlawson nolanlawson closed this May 7, 2015

@MattKunze

This comment has been minimized.

Copy link

MattKunze commented May 7, 2015

Confirmed that requiring pouchdb directly works now without any workarounds, but I'm still seeing the file size differences between requiring lib/index.js vs the dist bundle. The overhead is ~400kB (~150kB minified), so it's a pretty big difference

@nolanlawson

This comment has been minimized.

Copy link
Member

nolanlawson commented May 8, 2015

Can you debug Webpack to see what it's including? Browserify has disc, but I'm not sure if Webpack has anything similar.

@nolanlawson

This comment has been minimized.

Copy link
Member

nolanlawson commented May 8, 2015

Also the reason the dist bundle is so small is because we spent a lot of time optimizing our Browserify build to remove stuff we don't need. Someone now needs to go through the same dance for Webpack. :)

@MattKunze

This comment has been minimized.

Copy link

MattKunze commented May 8, 2015

Was there a PR where you optimized it for browserify? That might be a good place to start.

webpack lists the modules it's including, so it's easy to know what is being included. I'll see if there's a verbose option to browserify to get similar results that can be compared.

@nolanlawson

This comment has been minimized.

Copy link
Member

nolanlawson commented May 8, 2015

Can't think of a particular one; there were lots of little ones that built up over time.

Basically all of those optimizations are expressed in this code. Things like disabling leveldown and fs are due to places in the Node code where we use those modules on the Node side but not on the browser side, so we can safely disable them during browserification.

Other things are more subtle to browserify, such as the fact that we sub out version.js for version-browser.js to avoid browserifying the entire package.json file. Ditto for buffer, which was a weird case where we could only avoid browserifying the entire Node Buffer module by using that buffer-browser.js trick. We also sub out bluebird for lie because it's smaller and we only use the standard Promise A+ spec APIs anyway.

@daleharvey

This comment has been minimized.

Copy link
Member

daleharvey commented May 8, 2015

Yeh the package.json isnt huge, but any large increase in size is usually due to buffer

@dashed

This comment has been minimized.

Copy link
Contributor

dashed commented Jul 24, 2015

For posterity, webpack users, do:

module: {
    loaders: [
        // https://github.com/pouchdb/pouchdb/issues/3319
        {
            test: /\.json$/,
            loader: "json-loader"
        }
    ]
}
@nolanlawson

This comment has been minimized.

Copy link
Member

nolanlawson commented Jul 24, 2015

@dashed Could you add this to the "common errors" page? Might be really helpful to people.

@dashed

This comment has been minimized.

Copy link
Contributor

dashed commented Jul 24, 2015

Good call. I amended the errors page.

@nickcolley

This comment has been minimized.

Copy link
Member

nickcolley commented Jul 24, 2015

If we config webpack on this repo will it solve that problem?

@nolanlawson

This comment has been minimized.

Copy link
Member

nolanlawson commented Jul 24, 2015

@nickcolley No, webpack specifically says that this kind of stuff is supposed to be solved by the end-user. ¯\_(ツ)_/¯

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment