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

feat: add /how-to/detect-ipfs-on-web #1295

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

lidel
Copy link
Member

@lidel lidel commented Sep 29, 2022

This PR:

  • tweaks mermaid diagrams config to match IPFS brand book color guide and render a readable size, even on mobile
  • adds /how-to/detect-ipfs-on-web page with some best practices around detecting IPFS resources on the web, based on recent discussions with Brave team and our effort to streamline implementation there.

As per usual, apologies for any grammar errors ;-)

PR preview

👉️ https://github.com/ipfs/ipfs-docs/blob/feat/how-to-implement-support-in-a-web-browser.md/docs/how-to/detect-ipfs-on-web.md

Website preview (static)

👉️ https://bafybeicp2oota2okffb4u6ta6gzx46jnpyyzxdrrzpzivtbo4xv6sfefim.on.fleek.co/how-to/detect-ipfs-on-web/

To see how the final docs.ipfs.tech version would look, see Fleek preview link in the CI section below.

@filecorgi
Copy link
Contributor

  • Image optimization came back clean!
  • Vuepress build was successful!

@@ -0,0 +1,127 @@
# Detect IPFS on the Web
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Title should probably be more general, because is not just about detection. eg "Handling IPFS Content in Web User Agents" or something in that direction.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We already have "Addressing IPFS on the Web" so I went with similar title, but not feeling strong either way.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems like this document helps to bridge the web2 -> web3 gap by showing web2 apps/services how to use ipfs.

Maybe "Discovering IPFS content from web2" or something?


```mermaid
flowchart TD
Start[Top-level navigation event]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If diagram only handles top-level navigation events, should add a note because policy there is typically different than other load points. Maybe point also to the test page.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The path detection diagram is generic enough to be applied to subresource requests too – the usual limitations we experience are vendor specific (example: ipfs/ipfs-companion#1095).

TODO(note to self): think how to convey that. Probably change labels on diagram to be about all requests + add section explaining caveats around "top-level navigaton" and "subresource requests" on HTTP and ipfs://.

Comment on lines +86 to +99
- If "IPFS Gateway Redirect / Protocol Upgrade" feature is enabled, and the HTTP URL was a gateway one, redirect automatically to `ipfs://cid/path?query#hash`

### Mutable `/ipns/key/..`
- Display "Open via IPFS" button in UI
- Clicking it should open `ipns://key/path?query#hash` (preserving any `?query` or `#hash` from the original HTTP URL)
- If "IPFS Gateway Redirect / Protocol Upgrade" is enabled, and the original HTTP URL was a gateway one, redirect automatically to `ipns://dnslink/path?query#hash`
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's not clear what "IPFS Gateway Redirect / Protocol Upgrade" is exactly, nor how I enable/disable it. Is this a companion feature, or a pseudo-docs feature that consumers of this document would want to implement? If necessary, can we link to more information?

Comment on lines +70 to +77
IsCachedDNSLink ==>|YES| ValidDNSLink
IsCachedDNSLink -->|NO| IsHeaderPresent
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I feel like "IsCachedDNSLink" is an unnecessary addition to the large flowchart that could be removed to clean things up a little bit. Mentioning that the resolved DNSLink could be cached is something that could be clearer given a separate description. There are pros and cons for users caching the resolved DNSLinks that could be mentioned in that separate description (or separate DNSLinks doc?)

@@ -0,0 +1,127 @@
# Detect IPFS on the Web
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems like this document helps to bridge the web2 -> web3 gap by showing web2 apps/services how to use ipfs.

Maybe "Discovering IPFS content from web2" or something?

docs/how-to/detect-ipfs-on-web.md Outdated Show resolved Hide resolved
docs/how-to/detect-ipfs-on-web.md Outdated Show resolved Hide resolved

- Display "Open via IPFS" button in UI
- Clicking it should open `ipns://dnslink/path?query#hash` (preserving `?query` or `#hash` from the original HTTP URL)
- If "DNSLink Website Redirect / Protocol Upgrade" is enabled, redirect automatically to `ipns://dnslink/path?query#hash`
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ditto. it may be useful to mention a list of "barebones features" that builders reading this doc may want to implement, describe a brief functionality, and then link to those from these items.

It would also clean up these sub-bullet points by centralizing them

Comment on lines +98 to +108
- It is a good practice to internally cache the fact that domain has a valid DNSLink.
- TTL from TXT record can be used as a hint when to expire cache entry.
- Performance can be improved further by using cached flag and revalidating it asynchronously, without blocking browser navigation.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we should put an asterisk in the flowchart when talking about getting DNSLink, and reference this part

@ElPaisano
Copy link
Contributor

@lidel @SgtPooki @autonome is this PR still needed? Cleaning out some old PRs now.

@lidel
Copy link
Member Author

lidel commented Apr 21, 2023

@ElPaisano Yes, this is still needed, just had no bandwidth to land it :(

FYSA had some related discussions during IPFS Thing and alternative long term home for things like this could be "Integrations" section at https://specs.ipfs.tech (to be added), where we would have this flow, and also have things like ipfs/specs#360.
Then, on docs.ipfs.tech we would have a simplified version, without caching details, as noted by @SgtPooki above.

We could merge this as-is, so at least we have something, and then move to specs, once we have “integrations” section there, and only have a simplified diagram in docs.

@BigLep
Copy link
Contributor

BigLep commented Apr 26, 2023

+1 on merging so we have something, and the moving to specs when it's ready to house it. Can you tackle this please @ElPaisano ?

@aschmahmann aschmahmann marked this pull request as draft May 8, 2023 13:31
@aschmahmann
Copy link
Contributor

from @lidel: This is fine to merge, but also ok to keep as a draft until we have time to move this to the specs repo.

@ElPaisano ElPaisano removed the request for review from TMoMoreau August 29, 2023 20:22
This adds a page with some best practices around detecting IPFS
resources on the web, based on recent discussions with Brave team
and our effort to streamline implmenetation there.
@ElPaisano ElPaisano force-pushed the feat/how-to-implement-support-in-a-web-browser.md branch from 3ed7743 to 5020d3b Compare September 25, 2023 22:48
@ElPaisano ElPaisano marked this pull request as ready for review September 25, 2023 23:03
@ElPaisano
Copy link
Contributor

@lidel this is ready for review. Made some adjustments

Main thing that is not clear to me is this section https://bafybeiblvih4yq6cbkxd7tauokc6qaxvvsvraftj7wa6sza6qyt5ggdbha.on.fleek.co/how-to/detect-ipfs-on-web/#what-to-do-with-detected-paths (starting on line 88)

Given that "The goal of this page is to provide some suggestions and best practices for Web 2 web browsers and other user agents that are interested in adding support for content-addressed resources.", do you mean that any browsers supporting IPFS should implement an "Open via IPFS" button that displays one of the various path types listed? I think SgtPooki had similar comments.

Also, #1295 (comment) is still not clear to me

@ElPaisano ElPaisano self-assigned this Sep 25, 2023
@BigLep BigLep mentioned this pull request Nov 9, 2023
11 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
No open projects
Status: 🔎 In Review
Development

Successfully merging this pull request may close these issues.

7 participants