Skip to content
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

Add IPFS implementation selector to settings page #714

Closed
olizilla opened this issue Nov 26, 2018 · 11 comments
Closed

Add IPFS implementation selector to settings page #714

olizilla opened this issue Nov 26, 2018 · 11 comments
Assignees

Comments

@olizilla
Copy link
Member

We want to let users try out js-ipfs with the IPFS Desktop app, see: #601

The main priority of IPFS Desktop is to provide a clear onboarding process for casual users, so v1 will ship with go-ipfs as the default implementation, but for those in the know, we can offer up a drop/down / select element in the IPFS Desktop settings page in the Web UI, see: ipfs/ipfs-webui#883

IPFS Desktop Settings

IPFS Desktop is a thin wrapper around IPFS Web UI, so the UI work for showing the new setting would be in a PR against IPFS Web UI. The plumbing to make the change happen lives in IPFS Desktop. @hacdias please can you comment on how best to implement that?

IPFS Companion has a selector that let's you choose between using js-ipfs-api and an embedded js-ipfs node.

screenshot 2018-11-26 at 12 39 39

We want to offer a similar feature here in Desktop, but in this case, both go-ipfs and js-ipfs would be run as an external daemon process, which is less restrictive than running in a browser, so we can offer them up as equally valid alternatives. We don't need as much copy warning about the trade-offs, and can focus on just linking to more information about both of them.

js-ipfs is 2.6Mb minified. As this is an installed desktop app, we could initially keep things simple and bundle it directly. If folks have signifcant concerns about the app size, then we could consider doing the extra work of pulling down js-ipfs on demand. Thinking ahead, @hacdias has already done some thinking about a more general purpose "ipfs backend selector" that we can revisit, after we have a basic go/js toggle in place.

@olizilla
Copy link
Member Author

@phoniks are you up for tackling this one? ✨ :shipit:

@phoniks
Copy link

phoniks commented Nov 26, 2018

@olizilla - Stoked for the opportunity to pitch in.
@hacdias - looking forward to your input.

I'll start with the implementing the toggle in webui and then work on the plumbing here in desktop.

@hacdias
Copy link
Member

hacdias commented Nov 26, 2018

Go ahead! If you need something, just ping me @phoniks 😄

@olizilla just a thought I have: right now, Web UI settings button is only active if the daemon is running and we have permission to access the configurations.

As time goes, Web UI's settings now also incorporate Desktop settings which will eventually get more robust and must be accessible even when the daemon is off.

We should think about this a bit more. I had one idea where there could be some providers of the settings and if one was available, the button would be too, and it would only show the available provider.

Right now, for example, the user can't even change Web UI language if the daemon isn't running. But the user should be able to do so.

@phoniks
Copy link

phoniks commented Nov 27, 2018

@hacdias This is a pretty basic question, but how did you approach enabling the desktop options while you were developing the settings page in the webUI repo? The two solutions I thought of were 1. commenting out the code that hides the component in SettingsPage.js, or 2. changing the fetch url for the desktop's web ui so that it fetches the development copy. I figure you must have had this same issue, so you might have a more clever solution than either of these. The former seems fairly simple, but my first couple of attempts gave unexpected results (a blank white page). The latter seems elegant in it's own way, but I have a hunch that it's doing too much to solve such a simple issue. What do you think?

edit -
It occurs to me now that another route might be to have the build script for ipfs-desktop use my ipfs-webui repo instead of a remote repo.

@hacdias
Copy link
Member

hacdias commented Nov 27, 2018

@phoniks to build the Web UI + Desktop together:

  1. Clone https://github.com/ipfs-shipyard/ipfs-webui and npm start that
  2. Replace webui://- by http://localhost:3000 [here] (don't commit this line!)
  3. Now npm start desktop

You'll probably also find interesting this preload file which is executed before any other content on Web UI window

@phoniks
Copy link

phoniks commented Nov 30, 2018

Thanks @hacdias. Now, I'm facing something new, and it really just has to do with my ignorance about the underlying libraries. I'm at the point of trying to spawn a new daemon after the switch has been flipped and I realized that I'm not sure what I should provide as the path for the js implementation. Would the same path work, and would it be preferable to reuse that directory (~/.ipfs)? Is it possible to preserve one's identity when switching between go and js implementations, or are you a separate node depending on your implementation?

@olizilla
Copy link
Member Author

olizilla commented Dec 1, 2018

@phoniks good question! Yes js-ipfs is compatible with an IPFS_PATH directory created by go-ipfs if the default settings are used.go-ipfs supports some some experiemental features like using BadgerDB as the data store that aren't compatible with js-ipfs, but apart from that it should work.

Of note, when you run a js-ipfs daemon from the command-line today, by default it will create it's IPFS_PATH dir at ~/.jsipfs. you have to alter the config to get it to use the same defaults as go-ipfs, but that is due to change; js-ipfs will use a similar default config to go-ipfs soon.

@phoniks
Copy link

phoniks commented Dec 2, 2018

Thanks @olizilla that explanation really helped. I thought for sure that figuring out that config variable
would be the last major hurdle toward completing this feature, but as it turns out I get an error whenever I pass a 'js' type in the createDaemon function and I'm not able to sort it out. The error is:

TypeError: Cannot read property 'config' of null at config (~/ipfs-desktop/src/utils/daemon.js:68:33)

Upon further research it seems that the start function is not actually starting the daemon, but it's not throwing an error. I switched back to master branch and toggled to the js implementation just to check, and I get the error even on the clean repo. If I switch back to the go implementation then it boots right up. Is there a difference between the way the ipfsd-ctl module interacts with js-ipfs as opposed to go-ipfs?

How can I dig deeper to sort out where the issue is? the ipfs.start() call doesn't throw an error.

To recreate on a clean branch just set the "type" variable to 'js' in index.js and comment out the 3 lines in utils/daemon.js that explicitly prevent a non-go "type". You should get the above error indicating that ipfsd.api is null.

@phoniks
Copy link

phoniks commented Dec 2, 2018

Turns out it was a fairly obvious issue - the default flags being passed in to the ipfsd.start() call don't seem to be compatible with js-ipfs. They were '--migrate=true' & '--routing=dhtclient'. Not sure how these will affect node functionality, but now that I've got the this js node starting, I figure I'll just make a pull request and look for feedback on how these flags will affect the performance. I imagine we'll want to include some kind of warning about how the js implementation hasn't reached feature parity with go-ipfs, hopefully with specifics about what to expect if you do experiment with js-ipfs.

@olizilla
Copy link
Member Author

olizilla commented Dec 3, 2018

@phoniks good detective work! Please do open a PR, and you can put wip: at the start of the PR title so we all know it's a request for feedback. In general feel free to work through any ideas you have as wip PRs.

@hacdias
Copy link
Member

hacdias commented Jul 24, 2019

Closing this in favor of #601. They're basically the same and one depends on another.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants