Skip to content
This repository was archived by the owner on Mar 5, 2025. It is now read-only.

Make ethereum.js browserify compatible #114

Closed
dchambers opened this issue Mar 15, 2015 · 12 comments
Closed

Make ethereum.js browserify compatible #114

dchambers opened this issue Mar 15, 2015 · 12 comments

Comments

@dchambers
Copy link
Contributor

ethereum.js doesn't support NPM users that use browserify to make their programs available in the browser. At present, if you attempt to use browserify on an app that requires ethereum.js than you will find that it includes a Node.js implementation of XHR, rather than the browser's native XHR.

ethereum.js seems to use process.env.NODE_ENV, envify & unreachable-branch-transform to support having different code paths in Node.js and the browser. This seems much clunkier than using the features built-in to browserify, but it's worse than that since:

  1. If you don't set process.env.NODE_ENV = 'debug' you end up with a Node.js version of XHR.
  2. If you do set process.env.NODE_ENV = 'debug' than the program errors because bignumber.js is purposely not loaded in that case (something to do with Meteor.js?).

IMO, a number of changes should be made:

  1. envify & unreachable-branch-transform should be dropped.
  2. The browser field should be used to cause the native XHR package to be loaded when using browserify.
  3. bignumber.js should always be used, in both the browser and Node.js.

If you are happy in principle with these changes, then I'd be happy to raise a pull-request for this work?

@dchambers
Copy link
Contributor Author

It looks like somebody has already created an issue for this in xmlhttprequest.

@dchambers
Copy link
Contributor Author

httpplease looks like a nicer alternative to, and is just a thin wrapper around, xmlhttprequest. Additionally, it supports browserify/webpack users out-of-the-box.

Would you be happy for any pull-request to use this library?

@debris
Copy link
Contributor

debris commented Mar 15, 2015

Hi! Thanks for contribution 👍

  1. envify & unreachable-branch-transform should be dropped.
  2. The browser field should be used to cause the native XHR package to be loaded when using browserify.

I'm cool with that.

  1. bignumber.js should always be used, in both the browser and Node.js.

Do you want to bundle it up into output ethereum.js files? I'm not sure about this. I would like to get some input from @kumavis or @cubedro .

httpplease looks like a nicer alternative to, and is just a thin wrapper around, xmlhttprequest. Additionally, it supports browserify/webpack users out-of-the-box.

It looks nice, but does it support synchronous requests?

@dchambers
Copy link
Contributor Author

Hi! Thanks for contribution 👍

You're welcome, and thanks for Ethereum!

Do you want to bundle it up into output ethereum.js files?

Well, I was more thinking about allowing end-developers to browserify their entire apps, and this is how many/most? web developers are building their apps nowadays.

Browserify comes close (--standalone and --external) but doesn't actually have very good support for creating standalone UMD modules that don't include their external libraries. Instead, cjs-umd seems to be what people are recommending for this.

I could try updating the pull-request so that dist/ethereum.js no longer included bignumber.js if you preferred? Maybe you'd like to offer both alternatives?

It looks nice, but does it support synchronous requests?

In hindsight, I decided not to cloud issues by switching XHR libraries in the pull-request at the same time as changing other parts.

@kumavis
Copy link
Contributor

kumavis commented Mar 16, 2015

@dchambers
This was resolved before, maybe it broke again?
#87

@kumavis
Copy link
Contributor

kumavis commented Mar 16, 2015

can you show how you're using browserify? and include browserify version
should automatically read that package.json and apply the transforms such that XHR doesnt get shadowed.

@kumavis
Copy link
Contributor

kumavis commented Mar 16, 2015

I've been down your debugging path as well
#86
#36
driverdan/node-XMLHttpRequest#96

@kumavis
Copy link
Contributor

kumavis commented Mar 16, 2015

@debris regarding packaging dependencies with the build: I always do this because the guarantee that my code works hinges on the version of the external library thats used. Giving up that guarantee for a small optimization in kb of js is not worth it today.

@kumavis
Copy link
Contributor

kumavis commented Mar 16, 2015

@dchambers tho my solution was a minimal fix, I'd prefer to simplify the build by removing the transforms.

@dchambers
Copy link
Contributor Author

This was resolved before, maybe it broke again?
#87

Hi @kumavis, perhaps #87 is what caused the issue in the first place, since it seems to be about making sure that browserify works the same for external libraries as it does for the ethereum.js build itself, yet the ethereum.js build purposely prevents browserify from bundling all of the dependencies?

can you show how you're using browserify?

Let me get back to you on that...

@dchambers tho my solution was a minimal fix, I'd prefer to simplify the build by removing the transforms.

👍

@dchambers
Copy link
Contributor Author

can you show how you're using browserify?

Okay, @kumavis, I've created a simple ethereum-test-dapp repository that merely browserifies ethereum.js, adding no code of it's own — obviously, the idea here is that you would eventually start adding lots of app code in different files, plus more external libraries that would make the use of browserify worthwhile.

The ethereum-test-dapp repository only works at the moment because I've configured it to use my fork of ethereum.js in the package.json.

@debris
Copy link
Contributor

debris commented Mar 16, 2015

fixed in #120

@debris debris closed this as completed Mar 16, 2015
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants