Init ipfs-api or js-ipfs #320

olizilla · 2017-11-28T16:42:39Z

Provide a mechanism for swapping between embedded and external ipfs nodes.

Adds ipfsNodeType as an option which is expected to be either embedded or external for use in the proposed UI in #319

To allow js-ipfs and ipfs-api to be used interchangeably, I've tweaked add-on code to use global Buffer impl povided by browserify, and use ipfs.files.add instead of ipfs.add. Intialisation is pulled into there own modules so we've got the flexibility to tweak the returned objects in the future if we find we need to bridge temporary api divergence, or customise anything.

Fixes a bundling issue by adding prundupify as a browserify plugin... due to browserify/factor-bundle#51 (factor-bundle and regular browserify de-duping don't play well together, which became an issue when bundling js-ipfs)

This is bite-size chunk of a bigger change, so I figured, best to get your thoughts on it early. I'm looking at pulling the external api polling from ipfs-companion.js to the external.js wrapper, so it'll become more similar to the embeded.js wrapper, as both will have some clean up to do in the destroy function.

lidel

Very exciting! :-)

FYSA API polling logic was ported from legacy addon. It just gets and caches current peer count under state.peerCount. There was no better way to to infer Node state at a time than to write simple heuristic around this single metric (fetch fail → offline, 0 → no peers, > 0 → online etc).

As long as you keep this stuff working in some form:

ipfs-companion/add-on/src/lib/ipfs-companion.js

Lines 475 to 479 in 84ed233

    
           function updatePeerCountDependentStates (oldPeerCount, newPeerCount) { 
        
             updateAutomaticModeRedirectState(oldPeerCount, newPeerCount) 
        
             updateBrowserActionBadge() 
        
             updateContextMenus() 
        
           }

It should be safe to do even a heavy refactor.

lidel · 2017-11-28T22:26:47Z

package.json

@@ -68,9 +68,11 @@
  },
  "dependencies": {
    "choo": "6.6.0",
+    "ipfs": "^0.26.0",


Let's try to keep all versions locked, so that we don't have big divergence between npm and yarn, at least on the direct dependency level.

Yeah, for IPFS land, always use ~ at least. A minor version might have a breaking change (0.27 will)

olizilla · 2017-11-29T14:31:19Z

Toggling ipfs node type works in Brave!

Still need to re-work the offlne detection, but is 🚀

daviddias · 2017-11-29T15:05:02Z

🎆🎇 THAT IS SO RAD!!!!! 🎆🎇
👏🏽👏🏽👏🏽👏🏽

Is the ipfs-companion also capturing ipfs://?

olizilla · 2017-11-29T15:07:26Z

@diasdavid

olizilla · 2017-11-29T15:38:39Z

@diasdavid was there a trick to getting the chrome.protocol apis to be available in ipfs-inactive/browser-laptop#1 ? I've tried adding it as a requested permission, but I'm now seeing chrome.protocol as undefined, and trying to track down why.

daviddias · 2017-11-29T15:48:10Z

@olizilla it might have been to how the extension was being loaded internally. Could you check with the Brave team?

olizilla · 2017-11-30T09:22:56Z

@alanshaw solved it!

Needed manifestLocation set to component. I think https://github.com/brave/muon/blob/1fd29625480ea3a7b66bd71da11f7bcc49daf420/brave/common/extensions/api/_api_features.json#L35-L42 restricts the protocol api to this.

For future explorers, you have to load the plugin programatically in brave and pass in an additional arg to get access to privileged apis like chrome.protocol

In brave/browser-laptop you link the add-on into your app/extensions dir add the following to app/extensions.js

  // Enable IPFS
  extensionInfo.setState('ipfs', extensionStates.REGISTERED)
  loadExtension('ipfs', getExtensionsPath('ipfs'), undefined, 'component')

it's the component arg that gives it super powers.

olizilla · 2017-12-01T10:33:04Z

merge Refactor ipfs-companion so it no longer sets properties on the window #321 and fix conflicts
refactor offline check

TODOs plucked from #310 (comment)

it should have analogous, well-documented checkbox in the Preferences
short description of Embedded option in a tool-tip when user hovers cursor on it.
explicitly list "API" address (under "HTTP GATEWAY").

Add a quick fix to handle the stream/buffer return type discrepancy between js-ipfs-api and js-ipfs.

Currently a peerCount of -1 means we couldn't connect to the external ipfs proc. This updates the automatic gateway switching logic to use that signalling value to decide whether to switch to the public gateway. Zero peers means we're initialising, rather than we'er offline. This improves the ux when switching between embedded and external nodes. A more extensive change can be done later, wrapping the ipfs-api client with a cache, and emitting events for when ipfs state changes.

olizilla · 2017-12-11T14:33:57Z

Ok, i've made a smaller change around

refactor offline check

Currently a peerCount of -1 means we couldn't connect to the external ipfs proc.
This updates the automatic gateway switching logic to use that signalling value
to decide whether to switch to the public gateway. Zero peers means we're
initialising, rather than we're offline.

This improves the ux when switching between embedded and external nodes.

A more extensive change can be done later, wrapping the ipfs-api client with a
cache, and emitting events for when ipfs state changes.

I'm planning on adding a feature flag of some sort, so we can merge this PR without exposing the js-ipfs option to existing users just yet. I'm focused on getting this PR mergable, before tackling any more changes.

lidel · 2017-12-11T14:54:39Z

@olizilla thank you! The "0 == initializing rather than offline" change sounds good.

As soon we have the flag I'll do my best to find time to smoke-test and merge it. 👌

Allows us to merge and release new versions while we iterate on embedded ipfs.

daviddias · 2017-12-12T08:36:40Z

add-on/src/lib/ipfs-client/index.js

 const external = require('./external')
+const embedded = require('./embedded')


Some feedback from the last IPFS all hands was that embedded might confuse devs to think that know there is a window.ipfs in every page. Is that one of the goals too? It would be a good feature to have (optionally)

I think that would be simple to add. Is the idea that users would write scripts against window.ipfs but not bundle it with their script? It sounds a little bit like those browser extensions that add jquery to every page, so you can noodle around in the dev console.

Do people have a use-case in mind? T feel like we should be directing devs to the js-ipfs examples, to show them how to bundle it.

The difference between adding jquery and ipfs is that on one you would be just packing more code, the second you are opening more ports and doing more crypto which are both expensive operations.

The Metamask team has been injecting Web3 to every page to let devs use Ethereum directly if WebPages are there.

I feel this should be one of the options for the extension and until we have concrete use cases, just refer to an example on how to use it to seed it to developers.

Lots of benefits for embedding an already running IPFS node in webpages, too many to list here, but for starter, it makes startup times a lot better as nothing has to initialized, just start using it directly (node in background is already running so!). Performance of loading pages and loading content will be faster as well. Things can be shared via MFS between applications, as long as they agree on a standard structure. ID can follow between applications as well. Just a few reasons.

Very cool! I like the idea of making ipfs available to webapps and seeing what people do with it.

See #330, I created it to track window.ipfs effort :)

This! https://github.com/VictorBjelkholm/js-ipfs-in-the-browser

olizilla · 2017-12-12T08:42:03Z

I've added a minimal feature flag for the embedded ipfs work. Marking ipfs: false in the browser section of the package.json, causes browserify to stub out require('ipfs'), returning an empty object instead of js-ipfs. I've added an isJsIpfsEnabled test that checks for that, and used it to hide the fieldset in the options page, and the toggle switch in the browser action menu. It will also cause js-ipfs to be excluded from the bundle, which is nice.

Ideally this would be controlled by an env var like JS-IPFS-ENABLED=true. I'll make that work in a future PR, if it's needed. To make that work we'd need to extract the browserify build process to a js file and use the browserify api, so we can then parameterise the excludes option based on env...

alanshaw · 2017-12-12T08:46:22Z

👓

alanshaw

Can't wait for this to land!

alanshaw · 2017-12-12T08:48:26Z

add-on/_locales/en/messages.json

  "panel_quickUpload": {
-    "message": "Quick Upload",
+    "message": "Share files via IPFS",


@lidel what's the workflow for language changes like this, should we change back to English in the other language files to denote that it needs to be re-translated?

@alanshaw Yes, this is what should be done, but I think this step can be skipped now that we use Crowdin.

Crowdin will change them back to english after I upload new version of source locale (en), at least that is what happened when I've added new keys (other locales automatically got english stubs).

(I manually sync github<->crowdin before releases)

alanshaw · 2017-12-12T13:12:18Z

@lidel I think this PR is ready to merge if you'd like to take a final look? If we can get this in I'll rebase #323 and it'll also be ready for review & merge.

lidel

LGTM!

Upload screen looks neat!

lidel · 2017-12-12T23:37:54Z

Firefox users can try v2.1.1beta5, which includes these changes. 👍

daviddias · 2017-12-13T06:32:56Z

Woooooot! :D

daviddias · 2017-12-13T06:34:13Z

How can I switch to a js-ipfs node?

I don't see an option on preferences.

lidel · 2017-12-13T08:05:00Z

@diasdavid Oops, should've stated it more clearly: embedded js-ipfs it is Brave-only for now 🙂 (disabled in default build). Firefox beta is mostly for testing any regressions.
To enable it, you need to mark ipfs: true in the browser section of the package.json and rebuild via npm run yarn-build, then load it into Brave.
(Thanks to this build flag Firefox/Chrome versions do not need to ship with unused codebase of js-ipfs)

alanshaw · 2017-12-13T08:59:14Z

@diasdavid @lidel you have to remove "ipfs": false from the package.json, setting it to true won't work! We'll add a note to the README.

olizilla added 4 commits November 28, 2017 12:37

Factor out creation of ipfs node

e030a36

Pass opts through to ipfs clients

7913c2e

Use global Buffer and ipfs.files

4f2eba9

Remove notes

2033c83

olizilla mentioned this pull request Nov 28, 2017

JS IPFS fallback #310

Closed

lidel reviewed Nov 28, 2017

View reviewed changes

olizilla added 3 commits November 29, 2017 10:03

Fix destroy logic. Pin deps

f603614

Merge remote-tracking branch 'origin/master' into feat/ipfs-init

b1c6dbe

Wire up toggle node type

67f8003

daviddias mentioned this pull request Nov 30, 2017

[WIP] Redesign ipfs/ipfs-desktop#513

Closed

11 tasks

olizilla added 5 commits November 30, 2017 11:07

Add stringProtocolHandler detect & ipfs:// handler

c3ce095

Handle missing notifications api

00f913e

Fix stream error. Disable mimeType check

e0e17d8

Add basic mime-sniff impl

d3989ec

Extract logo component and use on quick upload

f440bb3

olizilla mentioned this pull request Dec 1, 2017

Refactor ipfs-companion so it no longer sets properties on the window #321

Merged

olizilla added 2 commits December 1, 2017 11:45

Merge branch 'remove-bg-page-api' into feat/ipfs-init

ffc36f8

Add API to status section. Add element titles.

eda6abf

Add a quick fix to handle the stream/buffer return type discrepancy between js-ipfs-api and js-ipfs.

olizilla changed the title ~~Init ipfs-api or js-ipfs~~ [WIP] Init ipfs-api or js-ipfs Dec 1, 2017

Add ipfsNodeType option

4c50055

hacdias mentioned this pull request Dec 1, 2017

WIP - Redesign ipfs/ipfs-desktop#516

Merged

20 tasks

Fix typo

8529944

olizilla mentioned this pull request Dec 11, 2017

Simplify UX Around The State Of Gateway Redirect #325

Closed

Merge remote-tracking branch 'upstream/master' into feat/ipfs-init

7ff69af

olizilla mentioned this pull request Dec 11, 2017

Error: version mismatch: expected v6, found v5 ipfs/js-ipfs#1071

Closed

Hide js-ipfs if not available.

84b2ada

Allows us to merge and release new versions while we iterate on embedded ipfs.

daviddias reviewed Dec 12, 2017

View reviewed changes

alanshaw approved these changes Dec 12, 2017

View reviewed changes

olizilla changed the title ~~[WIP] Init ipfs-api or js-ipfs~~ Init ipfs-api or js-ipfs Dec 12, 2017

olizilla mentioned this pull request Dec 12, 2017

Adds self served directory listings #323

Merged

cosmetic fixes before merge

a0c38aa

lidel approved these changes Dec 12, 2017

View reviewed changes

lidel merged commit 85106e9 into ipfs:master Dec 12, 2017

olizilla mentioned this pull request Dec 14, 2017

[WIP] Add landing pages #326

Closed

lidel mentioned this pull request Jan 15, 2018

Support for js-ipfs as a replacement for HTTP gateway #248

Closed

lidel mentioned this pull request Mar 6, 2018

Web Extension allows choice of daemon: go-ipfs or js-ipfs ipfs/in-web-browsers#40

Closed

alanshaw deleted the feat/ipfs-init branch March 6, 2018 12:29

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Init ipfs-api or js-ipfs #320

Init ipfs-api or js-ipfs #320

olizilla commented Nov 28, 2017 •

edited

Loading

lidel left a comment •

edited

Loading

lidel Nov 28, 2017

daviddias Nov 29, 2017

olizilla commented Nov 29, 2017 •

edited

Loading

daviddias commented Nov 29, 2017

olizilla commented Nov 29, 2017

olizilla commented Nov 29, 2017

daviddias commented Nov 29, 2017

olizilla commented Nov 30, 2017

olizilla commented Dec 1, 2017 •

edited

Loading

olizilla commented Dec 11, 2017

lidel commented Dec 11, 2017 •

edited

Loading

daviddias Dec 12, 2017

olizilla Dec 12, 2017 •

edited

Loading

daviddias Dec 12, 2017

victorb Dec 12, 2017

olizilla Dec 12, 2017

lidel Dec 12, 2017

olizilla Dec 12, 2017

olizilla commented Dec 12, 2017 •

edited

Loading

alanshaw commented Dec 12, 2017

alanshaw left a comment

alanshaw Dec 12, 2017

lidel Dec 12, 2017 •

edited

Loading

alanshaw commented Dec 12, 2017

lidel left a comment

lidel commented Dec 12, 2017

daviddias commented Dec 13, 2017

daviddias commented Dec 13, 2017

lidel commented Dec 13, 2017 •

edited

Loading

alanshaw commented Dec 13, 2017

	function updatePeerCountDependentStates (oldPeerCount, newPeerCount) {
	updateAutomaticModeRedirectState(oldPeerCount, newPeerCount)
	updateBrowserActionBadge()
	updateContextMenus()
	}

		const external = require('./external')
		const embedded = require('./embedded')

Init ipfs-api or js-ipfs #320

Init ipfs-api or js-ipfs #320

Conversation

olizilla commented Nov 28, 2017 • edited Loading

lidel left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

olizilla commented Nov 29, 2017 • edited Loading

daviddias commented Nov 29, 2017

olizilla commented Nov 29, 2017

olizilla commented Nov 29, 2017

daviddias commented Nov 29, 2017

olizilla commented Nov 30, 2017

olizilla commented Dec 1, 2017 • edited Loading

olizilla commented Dec 11, 2017

lidel commented Dec 11, 2017 • edited Loading

Choose a reason for hiding this comment

olizilla Dec 12, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

olizilla commented Dec 12, 2017 • edited Loading

alanshaw commented Dec 12, 2017

alanshaw left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lidel Dec 12, 2017 • edited Loading

Choose a reason for hiding this comment

alanshaw commented Dec 12, 2017

lidel left a comment

Choose a reason for hiding this comment

lidel commented Dec 12, 2017

daviddias commented Dec 13, 2017

daviddias commented Dec 13, 2017

lidel commented Dec 13, 2017 • edited Loading

alanshaw commented Dec 13, 2017

olizilla commented Nov 28, 2017 •

edited

Loading

lidel left a comment •

edited

Loading

olizilla commented Nov 29, 2017 •

edited

Loading

olizilla commented Dec 1, 2017 •

edited

Loading

lidel commented Dec 11, 2017 •

edited

Loading

olizilla Dec 12, 2017 •

edited

Loading

olizilla commented Dec 12, 2017 •

edited

Loading

lidel Dec 12, 2017 •

edited

Loading

lidel commented Dec 13, 2017 •

edited

Loading