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.

Sign up for GitHub

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

Jump to bottom

Switch to upstream builds of is-ipfs and js-ipfs-api #152

Closed
lidel opened this issue Sep 26, 2016 · 3 comments
Closed

Switch to upstream builds of is-ipfs and js-ipfs-api #152

lidel opened this issue Sep 26, 2016 · 3 comments
Labels
kind/maintenance Work required to avoid breaking changes or harm to project's status quo
Milestone
v2.0.0

Comments

@lidel
Copy link
Member

lidel commented Sep 26, 2016 •
edited
Loading

It is not a good practice to maintain own build of browserified is-ipfs.
It makes AMO reviews harder and introduces noise to the code hosted in add-on repository.

Just like with js-ipfs-api (#151), the WebExtension should load official browserified artifact from IPFS mirror of NPM (it is now safe to do so due to SRI validation).
@lidel lidel added this to the v2.0.0 milestone Sep 26, 2016
@lidel lidel added the kind/maintenance Work required to avoid breaking changes or harm to project's status quo label Sep 26, 2016
@lidel
Copy link
Member Author

lidel commented Oct 1, 2016 •
edited
Loading

As mentioned in ipfs-shipyard/is-ipfs#8 (comment) browserified version works fine. 👍

Unfortunately, when I load both is-ipfs and js-ipfs-api (#151), the latter one does not load due to error:

Error: only one instance of babel-polyfill is allowed

Related bugs (Aergir is used by both is-ipfs and js-ipfs-api):

I'll probably have to maintain my own "merged" artifact until ipfs/aegir#36 is fixed. It is unfortunate, I really hoped to use upstream artifacts without any JS shenanigans..

@lidel lidel changed the title Switch to upstream build of is-ipfs Switch to upstream builds of is-ipfs and js-ipfs-api Oct 1, 2016
This was referenced Oct 1, 2016
Closed
Closed
@lidel
Copy link
Member Author

lidel commented Oct 10, 2016

Short update:

It does not seem to be fixed upstream any time soon, so tried to build "merged" browser bundle with vanilla browserify – my hopes were that it would "just work" 😞

Entry point lib-npm/window.js:

window.IsIpfs = require('is-ipfs')
window.IpfsApi = require('ipfs-api')

Build command to expose libraries under window namespace:

browserify lib-npm/window.js --standalone window -d > lib-npm/deps.js

It works for is-ipfs, but ipfs-api code fails due to missing fs implementation:

TypeError: fs.readFileSync is not a function

I think a safe approach would be to get familiar with webpack and port the config from AEgir, which proven to produce browser bundle that runs in WebExtension without any problems.

@lidel
Copy link
Member Author

lidel commented Oct 16, 2016 •
edited
Loading

Shower 🚿 thought:

The only issue is that a window object can't be shared between "browserified" artifacts. What if we just avoid dealing with javascript alchemy from the very beginning and load every artifact in a separate background page?

Problem: as of now, WebExtension supports only one background page in its manifest.json:

"background": {
        "page": "background/init.html"
    },

Solution: use iframes.
Every iframe comes with a separate window namespace, which is what we want.

As a PoC I've put every component in a separate <iframe>-based sandbox.

background/init.html:

<!DOCTYPE html>
<meta charset="utf-8">
<iframe name="ipfsApiSandbox" src="sandbox/ipfsApi.html"></iframe>
<iframe name="isIpfsSandbox" src="sandbox/isIpfs.html"></iframe>
<script src="init.js"></script>

background/init.js:

let ipfsApi = window.frames['ipfsApiSandbox'].IpfsApi
let isIpfs = window.frames['isIpfsSandbox'].IsIpfs

Aaand it works perfectly 🚀

All I had to do was to make sure iframes finish loading before I access their window objects.
Finally, I am able to use upstream artifacts in WebExtension without any modification.

lidel added a commit that referenced this issue Oct 16, 2016
@lidel
FIX: sandboxed loading of browserified libraries
b6dbbde 
- background: https://git.io/vPiVp
- closes #152
@lidel lidel closed this as completed Oct 16, 2016
lidel added a commit that referenced this issue Nov 8, 2016
@lidel
chore: ipfs-api 11.1.0 (remove sandbox)
c999569 
This version removed babel-polyfil, which enabled us to remove
sandbox workaround described in #152
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
kind/maintenance Work required to avoid breaking changes or harm to project's status quo
Projects
None yet
Milestone
v2.0.0
Development

No branches or pull requests
1 participant
@lidel