-
Notifications
You must be signed in to change notification settings - Fork 1.2k
feat: Expose ipfs utils and add an example on how to run ipfs in the browser #525
Conversation
nice! the only thing I would do different in the example would be: const IPFS = typeof Ipfs !== 'undefined' ? Ipfs // browser global
: typeof require === 'function' ? require('ipfs') // cjs require
: throw new Error('could not load IPFS') to me, the |
@heavyk How about this? |
I like it :) it certainly clarifies the docs a bit. it's by no means extensive, but that would have certainly been helpful to me when I began. I ended up looking through a bunch of tests to figure out what was going on, so yea |
I would highly recommend to merge this PR asap. It improves our docs 200%. @diasdavid @dignifiedquire can you verify that the startup sequence code is what is currently needed? |
Thanks for the PR @interfect! |
Is there anything I still ought to do to get this merged in? |
@@ -110,7 +110,109 @@ The last published version of the package become [available for download](htt | |||
|
|||
### Examples | |||
|
|||
> **Will come soon** | |||
Below are spome examples of JavaScript IPFS in action. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add a ToC for the examples provided, so that are easily seekable.
> **Will come soon** | ||
Below are spome examples of JavaScript IPFS in action. | ||
|
||
#### Startup (in Node.js or browser) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's break the "Creating an instance" into two parts
"Create an IPFS instance - Node.js"
"Create an IPFS instance - Browser"
Just to avoid that 'scary' require in line 121
|
||
#### Startup (in Node.js or browser) | ||
|
||
There's still a bit of work required to start up a node in a robust way (i.e. without requiring the user to manually `js-ipfs init`). Here is one way to do it. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This sentence might be confusing to new users. "What is starting a node in a robust way"? Why is doing 'init' not robust?
|
||
// Make an IPFS node | ||
var ipfs = new IPFS() | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As said above, let's make the code till here two sections "Node.js and Browser"
// Make an IPFS node | ||
var ipfs = new IPFS() | ||
|
||
// Init a repo in the given IPFS node it if hasn't got one already. Calls the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
From here down, let's create a section "Init and start the node"
ipfs.config.set('Addresses.Swarm[1]', star_addr, (err) => { | ||
if (err) { | ||
throw err | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This also requires the signalling server of libp2p-webrtc-star to be up. Which makes me think that maybe we should have one always running for people to do demos that goes with the default config.
@lgierth how does this sound?
We have two options:
a) If we get a signalling server always up, then we should make WebSockets and webrtc-start be the default config values for a browser node (the same way we have the TCP Sockets)
b) If we don't get the signalling server always up. We should set the default addrs to be just websockets for browser and have a separate example that shows how to turn on WebRTC.
```javascript | ||
ipfs.files.cat('QmNRCQWfgze6AbBCaT1rkrkV5tJ2aP4oTNPb5JZcXYywve', | ||
(err, content_stream) => { | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
extra new line
@@ -187,6 +289,8 @@ IPFS Core is divided into separate subsystems, each of them exist in their own r | |||
|
|||
IPFS Core is the entry point module for IPFS. It exposes an interface defined on [IPFS Specs.](https://github.com/ipfs/specs/blob/ipfs/api/api/core/README.md) | |||
|
|||
This particular implementation also provides some extensions to assist with usage in a browser, such as the `Buffer` type, required as input for some API methods, being available as `ipfs.Buffer` on every `IPFS` object. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's make it more of a disclaimer
#### Need the Buffer class?
IPFS exposes the Buffer class in every ipfs instance, so that you can create buffers and add them to IPFS just like if you were using it in Node.js.
You can get it at `ipfs.Buffer`
// Export Buffer to somewhere where API consumers can get at it, for when | ||
// we're running in a browser but still expect Buffer instances to be passed | ||
// in. | ||
this.Buffer = Buffer |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just say expose Buffer for browser applications.
@interfect excellent work! Thank you a lot for this, having someone building a tutorial for js-ipfs and seeing all of the hurdles you had to go through to get IPFS running... I'm sorry :( We can definitely make it better and make this tutorial way simpler, in fact, I want to have it sooner than later, would you like to make this PR a little bit by improving the API to start IPFS? It should be something like:
So that a
Would do the |
beep boop :) This PR needs a rebase. May I also ask for it to:
Thank you! :) |
Buffer
and document browser usage
Sorry for kind of vanishing. I don't think I want to try to take on refactoring the API myself, or at I will rebase this and try to split up the example code into node and On Mon, Nov 14, 2016 at 2:58 AM, David Dias [email protected]
|
Sounds good to me @interfect we can conquer this one by parts :) Also happy to explain you more in depth of how the internals work to get you more comfortable with it, through chat in IRC/Github or a hangout |
ff978f1
to
f84438a
Compare
@diasdavid I think I have addressed your comments. I fixed up the comments in the actual code, added an example under examples, and simplified the ugly IPFS-finding logic. My example in the README is getting pretty big, since I included more of a writeup on why it is like it is. Maybe it wants to move somewhere else? If so, where would that be? |
@interfect please move the example to the examples folder so that it is easy to run, making the readme a bit more lean (#525 (comment)) |
@@ -39,6 +39,11 @@ Consult the [Roadmap](/ROADMAP.md) for a complete state description of the proje | |||
- [HTTP-API](#http-api) | |||
- [IPFS Core examples (use IPFS as a module)](#ipfs-core-examples-use-ipfs-as-a-module) | |||
- [Create a IPFS node instance](#create-a-ipfs-node-instance) | |||
- [Find IPFS in Node.js](#find-ipfs-in-nodejs) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't believe this will be needed as with the examples we have right now on https://github.com/ipfs/js-ipfs/tree/master/examples, but feel welcome to complement them.
@@ -39,6 +39,11 @@ Consult the [Roadmap](/ROADMAP.md) for a complete state description of the proje | |||
- [HTTP-API](#http-api) | |||
- [IPFS Core examples (use IPFS as a module)](#ipfs-core-examples-use-ipfs-as-a-module) | |||
- [Create a IPFS node instance](#create-a-ipfs-node-instance) | |||
- [Find IPFS in Node.js](#find-ipfs-in-nodejs) | |||
- [Find IPFS in the Browser](#find-ipfs-in-the-browser) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't believe this will be needed as with the examples we have right now on https://github.com/ipfs/js-ipfs/tree/master/examples, but feel welcome to complement them.
@@ -48,6 +53,7 @@ Consult the [Roadmap](/ROADMAP.md) for a complete state description of the proje | |||
- [Files API](#files-api) | |||
- [Swarm API](#swarm-api) | |||
- [libp2p API](#libp2p-api) | |||
- [Need the Buffer class?](#need-the-buffer-class) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe replace this by "Domain data types"
, so that it includes things like multiaddr, multihash, etc.
This lets you construct a Buffer even in browser code that doesn't have access to the Node Buffer global, so that you can pass it back to IPFS API calls that want a Buffer. Add some usage documentation Mentions the `ipfs.Buffer` field you can use to stamp out `Buffer`s so you can actually insert stuff given just an `IPFS` instance and no Node.js APIs in scope. Also add examples for using IPFS in browser with a script tag, and for using libp2p-webrtc-star.
OK, I've gone and kicked most of my changes out of the main README and into a couple of examples: one just for IPFS in a browser with script tags, and one for IPFS in a browser with libp2p-webrtc-star. @diasdavid could you please review this again? I don't know what's up with nodejs 4 in the tests; I changed ~3 lines of actual code. |
@diasdavid Is there anything else you want changed? |
IPFS exposes the Buffer class in every ipfs instance, so that you can create buffers and add them to IPFS just like if you were using it in Node.js. | ||
|
||
You can get it at `ipfs.Buffer` | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since devs will start relying on this, let's make sure it is complete by putting these 'Types' behind, ipfs.types
and adding things like multiaddr, multihash and so.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've added a ipfs.types.
at https://github.com/ipfs/js-ipfs/blob/master/src/core/index.js#L25 that you can expand to include all the IPFS types.
@@ -208,6 +245,12 @@ Every IPFS instance also exposes the libp2p API at `ipfs.libp2p`. The formal int | |||
- [libp2p-ipfs-nodejs](https://github.com/ipfs/js-libp2p-ipfs-nodejs) | |||
- [libp2p-ipfs-browser](https://github.com/ipfs/js-libp2p-ipfs-browser) | |||
|
|||
#### Domain data types | |||
|
|||
IPFS exposes the Buffer class in every ipfs instance, so that you can create buffers and add them to IPFS just like if you were using it in Node.js. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Rather explain that we expose a set of data types directly from the ipfs instance, so that devs don't have to require/import them again.
@@ -225,6 +268,8 @@ Every IPFS instance also exposes the libp2p API at `ipfs.libp2p`. The formal int | |||
|
|||
### Run Tests | |||
|
|||
#### Block Service | |||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe this was a bad c&p
<p>Note that opening two tabs of this page in the same browser won't work well, because they will share node configuration. You'll end up trying to run two instances of the same node, with the same private key and identity, which is a Bad Idea.</p> | ||
<div id="status">Node status: offline</div> | ||
</body> | ||
</html> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This example is missing loading IPFS from a script tag with spawning a webrtc-star sig server
@interfect will you be looking into finish this PR soon? |
Honestly with all the rounds of requested changes it's not something I look
forward to working on. It's still on my to do list, but I keep putting it
off.
On Feb 1, 2017 8:10 AM, "David Dias" <[email protected]> wrote:
@interfect <https://github.com/interfect> will you be looking into finish
this PR soon?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#525 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AAt5tuXxeCJfDGNGVIFGEHozjxLpOMLfks5rYK6MgaJpZM4KZXhn>
.
|
Looks like this has been picked up by some other PRs, so I'm going to close it. |
Trying to use IPFS in a browser, I ran into trouble actually invoking
ipfs.files.add
, because it wanted me to pass aBuffer
, but no implementation ofBuffer
was available in the browser namespace. This lets theBuffer
implementation used internally inside theIPFS
class get exposed as a field on instances, so you can invokeipfs.files.add
given only anIPFS
object and no other Node globals.Also, I've included some examples demoing use in a browser, and featuring this new exported
Buffer
type. It took me a long time to work out the incantations to use, so I feel obliged to put them somewhere prominent.If there was an official bootstrap
libp2p-webrtc-star
signaling server, I would use that instead of127.0.0.1
in my example. If you don't have at least one address on your node, including a signaling server IP, you don't get the transport added to the node and then you can never dial out through any signaling server. So I figured I had to add it in the example, to not confuse people over why they couldn't use the transport.Closes #523. Also should address #513. Also may help with #509, since I definitely have it loading up in a browser.