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 support for Chrome Apps and WebExtensions #3946

Merged

Conversation

lidel
Copy link
Contributor

@lidel lidel commented Apr 26, 2019

Stream detection via stream instanceof Stream does not work correctly in browser contexts such as Chrome Apps or WebExtensions in browsers exposing chrome.sockets, especially when different stream polyfils are used and mixed together.

This change replaces instanceof Stream checks with feature-detection of stream contract, enabling Hapi to be used in browserified form in Chrome Apps and other browser environments.

cc ipfs/ipfs-companion#716

@hueniverse
Copy link
Contributor

First, I am not sure I want to support this use case. Can you give me an example where doing so would make sense? What kind of app would you be building with it?

Second, experience tells me this is not the only thing stopping it from working properly. We've gone through this with joi and other modules that required a lot of changes to work properly.

@lidel
Copy link
Contributor Author

lidel commented Apr 29, 2019

Real live example: js-ipfs in Brave browser (ipfs/ipfs-companion#716)

js-ipfs is using Hapi to expose HTTP2IPFS Gateway. Brave whitelisted our browser extension to have access to chrome.sockets and we are able to start HTTP2IPFS Gateway from within WebExtension context, without installing any additional software.

We tested it in practice and it works as expected (ipfs/ipfs-companion#664 (comment)). We use chrome-net and http-node to polyfill calls to net and http expected by Hapi.

The only remaining issue with Hapi was the Stream detection, and change from this PR solved that (we use code form this PR).

Stream-related entries from test/response.js pass, but I'm happy to add any additional tests you feel are necessary.

@hueniverse
Copy link
Contributor

Thanks for the background. I hope to get to it this week and let you know if/how I can support it.

@lidel
Copy link
Contributor Author

lidel commented Jul 10, 2019

Anything I can do to help with evaluating this?

@lidel lidel force-pushed the fix/streams-in-chrome-app-environment branch from ccbf84b to fb8626f Compare September 12, 2019 15:57
Detection via `stream instanceof Stream` does not work correctly in browser context,
especially when different polyfils are used and mixed together.

This change replaces instanceof check with feature-detection of stream-like objects.
It enables Hapi to be used in Chrome Apps (browser environments with additional chrome.sockets APIs)

Real life example:
ipfs/ipfs-companion#664
@lidel lidel force-pushed the fix/streams-in-chrome-app-environment branch from fb8626f to 0d73f8d Compare September 12, 2019 16:54
@hueniverse hueniverse self-assigned this Sep 13, 2019
@hueniverse hueniverse added the feature New functionality or improvement label Sep 13, 2019
@hueniverse hueniverse added this to the 18.3.3 milestone Sep 13, 2019
@hueniverse hueniverse merged commit b3535a4 into hapijs:master Sep 13, 2019
hueniverse added a commit that referenced this pull request Sep 14, 2019
lidel added a commit to ipfs/ipfs-companion that referenced this pull request Oct 9, 2019
Release includes support for Chrome Apps and WebExtensions:
hapijs/hapi#3946
lidel added a commit to ipfs/ipfs-companion that referenced this pull request Oct 11, 2019
Release includes support for Chrome Apps and WebExtensions:
hapijs/hapi#3946
@lock
Copy link

lock bot commented Jan 9, 2020

This thread has been automatically locked due to inactivity. Please open a new issue for related bugs or questions following the new issue template instructions.

@lock lock bot locked as resolved and limited conversation to collaborators Jan 9, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
feature New functionality or improvement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants