README: improve browser setup advice

[dcousens]

As per #314, this updates the README to include more up-to-date and idiomatic build advice for users who want to use the library from the browser.

Suggestions welcome and appreciated.

[weilu]

ha! you beat me at it and went the other way haha

[weilu]

I would like to keep the compile script as then dependencies including build tools are managed by NPM. #314 (comment)

[dcousens]

Technically the API is not [fully] capable of being used without the external dependencies, as such, the resultant build is not really usable unless you force the user to customize src/index.js.

[weilu]

With the current loosened type checking in types.js users are no longer forced to customize index.js. They can and should browserify external dependencies in separate bundles. I believe this is not a problem we should solve, as it is caused by the absence of a proper dependency manager when the user chooses to use a bundled, minified script directly in browser.

[dcousens]

@kyledrake I'm personally not a fan of this remaining since it provides a user with a bitcoinjs-lib that isn't entirely usable as they would be missing particular dependencies to use some parts of our API.
We do allow duck typing which means they could include the other dependencies as individual minified files if they so wish; but it does encourage a bad practice IMHO.

Removing it forces them to use the browserify process themselves, there in learning how to use our API in its entirety from the get go.

[kyledrake]

As I understand it, this PR removes it? I don't have a strong opinion about keeping it, I think Wei's instructions are better.

[dcousens]

If not what currently exists, I'm happy to merge the following from the #314.

Browser

bitcoinjs-lib works with browserify. If you want to compile a minified version with a global object in the browser, here is how.

From the repository:

# Checkout the latest tagged version of the source code
git clone https://github.com/bitcoinjs/bitcoinjs-lib.git
cd bitcoinjs-lib
git checkout `git describe`

# Install dependencies & compile `bitcoinjs-min.js`
npm install
npm run-script compile

Alternatively, from NPM:

npm install bitcoinjs-lib
cd node_modules/bitcoinjs-lib
npm run-script compile

After loading the compiled bitcoinjs-min.js file in your browser, you will be able to use the global bitcoin object.

If you want to customize exported modules in the bundled, update src/index.js and run npm run-script compile.

[dcousens]

The above suggested changes by @weilu (as I have quoted from her post elsewhere) don't make any enough sense.
It suggests two possible code paths, both of which require npm anyway removing any benefit to using git in the earlier stages.

The entire thing boils down to "run npm run compile inside the bitcoinjs-lib package root".

I'm happy to add that step to what I've written as an alternative to the more advanced usage, but I think both the advanced and basic usage are important to avoid users constantly asking (#231 (comment), https://github.com/bitcoinjs/bitcoinjs-lib/pull/257/files, #308 (comment)).

And so we can avoid advice such as #228 (comment) misleading people to thinking that will allow them to use the library fully without further modifications.

[dcousens]

ping @kyledrake

[weilu]

The reason for having both the git path and npm path is as I said on IRC: "I thought some devs might be more comfortable with git for updating/upgrading. Say they change the index.js to add/remove things, they can stash then git pull to update. With npm their changes will be discarded."

And so we can avoid advice such as #228 (comment) misleading people to thinking that will allow them to use the library fully without further modifications.

Again, it's an issue caused by the lack of proper dependency manager when one uses the bundled code by itself without browserifying the rest of their code. IMHO modifying index.js to include dependencies is a convenient hack. Instead what they should be doing is browserifying and exporting things like Buffer independently as it's not something that should live under the namespace bitcoin. We can add a warning but it's their problem not our responsibility.

[dcousens]

exporting things like Buffer independently as it's not something that should live under the namespace bitcoin. We can add a warning but it's their problem not our responsibility.

In the context of its only use for them being the interaction with this library. Maybe it is relevant?

I don't disagree with your other points though. I'm just trying to reduce the complaining.

[weilu]

We do resolve our internal dependency on Buffer. Devs don't need buffer to interact with this library until they want to use the endpoints that accepts buffer as an argument. Buffer is not a function invented by bitcoin. It's very much generic and can be used on its own in applications or by other libraries. This library makes an assumption that it is available, because it is true for node and browserify.

[kyledrake]

I'm +1 with the idea of showing developers how to compile their own, and provide their own dependencies as needed. If we show them an example of this in the README so they can quickly get started compiling a version that has the interfaces they need, that seems fine.

Perhaps in the future, things may cool down a bit and then we could provide access to the internals if we expect them to be stable for a long time. But really, I think people should be learn how to use Browserify and know how to use it properly, and know how to compile the deps they need themselves.

I don't disagree with your other points though. I'm just trying to reduce the complaining.

+1 to letting them complain. If it keeps happening, let's just add something to the README about it how we're not changing it.

Debate is great, but the color of the bike shed is not really the kind of debate that moves the library forward. We're using Node.js and Browserify for developing this, it runs on everything, it's easy to test, we think it's great for producing high quality JS software, we're not changing it, you'll eventually need to know how to use it anyways, so learn how to use it.

[adatta02]

I just ran up against the "no Buffer in browser" issue and wanted to add my 2 cents.

After reading through the discussion related to using bitcoinjs-lib in a browser, I think the overarching issue is that the docs suggest that running "npm run-script compile" will leave you with a fully functionaly library which isn't the case.

I think re-writing the "Browser" section on the README to reflect the points discussed in the issues related to browser use along with a note mentioning that core NodeJS modules wont be exported by default with the "full export" code from #308 (comment) included would help immensely.

[dcousens]

@kyledrake

We're using Node.js and Browserify for developing this, it runs on everything, it's easy to test, we think it's great for producing high quality JS software, we're not changing it, you'll eventually need to know how to use it anyways, so learn how to use it.

Which is one of the main reasons I feel that providing npm run compile is misleading and wrong [in its current form].

[weilu]

@dcousens @kyledrake If you guys are happy with the changes in this PR go ahead merge it. My problems remain as

1. the use of browserify directly without having its version managed by a dependency manager.
2. buffer is a generic function that should not be living under the bitcoin namespace

I feel that providing npm run compile is misleading and wrong [in its current form].

I feel strongly about keeping npm run compile for the above mentioned two reasons. If we were to keep it, and accompany it with what @adatta02 suggested, @dcousens would that work for you?

a note mentioning that core NodeJS modules wont be exported by default with the "full export"

[dcousens]

@weilu I've updated the instructions.

I believe I've kept true to your intentions,

the use of browserify directly without having its version managed by a dependency manager.

see df50393

buffer is a generic function that should not be living under the bitcoin namespace

see 1d9bc2c

I think the above makes it abundantly clear we are just helping the user understand how to use browserify, and not just providing a convenient hack.

[coveralls]

Coverage remained the same when pulling 1d9bc2c on browserdocs into ecadda0 on master.

[weilu]

it will complain that bitcoinjs-lib is not found as it has not yet been installed through npm

[dcousens]

Same with bs58, and any others.

[weilu]

So? the instruction is still incomplete

[dcousens]

Actually, it has been installed.

This is under the ## Setup header, with ## Installation above it.
Therefore, at this stage, it is assumed they have bitcoinjs-lib installed.

[coveralls]

Coverage remained the same at 98.25% when pulling 75ca385 on browserdocs into abf870f on master.

[weilu]

mkdir node_sandbox
cd node_sandbox/
npm init
npm install bitcoinjs-lib
browserify foobar.js -s foobar > dist/foobar.js
-bash: dist/foobar.js: No such file or directory

[weilu]

browserify foobar.js -s foobar > bundle.js
Error: Cannot find module 'bs58' from '/Users/luwei/workspace/node_sandbox'
    at /usr/local/lib/node_modules/browserify/node_modules/resolve/lib/async.js:50:17
    at process (/usr/local/lib/node_modules/browserify/node_modules/resolve/lib/async.js:119:43)
    at /usr/local/lib/node_modules/browserify/node_modules/resolve/lib/async.js:128:21
    at load (/usr/local/lib/node_modules/browserify/node_modules/resolve/lib/async.js:60:43)
    at /usr/local/lib/node_modules/browserify/node_modules/resolve/lib/async.js:66:22
    at /usr/local/lib/node_modules/browserify/node_modules/resolve/lib/async.js:21:47
    at Object.oncomplete (fs.js:107:15)

[weilu]

@dcousens Did I fail at reading the readme?

[dcousens]

Nope,

@weilu try now after 642315e

[dcousens]

Again, how do you control versions of those libraries?

That is a good point. I guess it becomes obvious in your examples, but this problem is independent of this PR as it also affects the node set up advice.

How do we control users using bigi@latest in conjunction with our API instead of what we use internally?
The best we can do is either simplify our API such that common JS types are used for data interchange or we ask the users to verify their dependencies against what we use internally.

[dcousens]

I'm going to merge this, but the point brought up by @weilu is quite valid.
I'll summarize and it make another issue.

How do we control users using bigi@latest in conjunction with our API instead of what we use internally?
The best we can do is either simplify our API such that common JS types are used for data interchange or we ask the users to verify their dependencies against what we use internally.