svecosystem / runed

Magical utilities for your Svelte applications (WIP)
https://runed.dev
MIT License
387 stars 21 forks source link

feat: add useNetworkStatus #117

Open michal-weglarz opened 3 weeks ago

michal-weglarz commented 3 weeks ago

Hi all đź‘‹ I thought it might be useful to have this useNetworkStatus utility to get the current status of the network connection at any time. It could be used to inform users that they have a bad connection, or that they have temporarily lost access to the internet altogether. I also added the updatedAt property, which can be used, for example, to show how much time users were offline before coming back online (please, see the attached demo).

It's heavily inspired by useNetworkState from React-Hookz/web.

This hasn't been discussed in a separate feature request yet, so feel free to reject this PR if you think this utility doesn't belong in the library.

Demo in Chrome:

https://github.com/svecosystem/runed/assets/37783495/4271b5de-1197-41a5-afc7-9896a48b6910

Demo in Safari/Firefox:

https://github.com/svecosystem/runed/assets/37783495/7694a5f3-82f3-460b-87f2-dc9321b5f149

The returned status always includes online and updatedAt. Other properties are returned based on the NetworkInformation interface and depend on your browser's compatibility.

changeset-bot[bot] commented 3 weeks ago

⚠️ No Changeset found

Latest commit: baaff333493699dd9420a27c97fd4f19efd20742

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

github-actions[bot] commented 3 weeks ago
built with Refined Cloudflare Pages Action

⚡ Cloudflare Pages Deployment

Name Status Preview Last Commit
runed âś… Ready (View Log) Visit Preview 14c85cba4a672a9e5d588421a30b6579816fcdd7
huntabyte commented 2 weeks ago

Part of me wonders if we should just include a previous property and just provide that as an API out of the box. I'm also wondering if we should expose an isConnectionSupported property as well so users can better conditionally handle situations where none of the other properties will be defined.

What do you all think?

Great job with the implementation @michal-weglarz!

TGlide commented 2 weeks ago

Why the previous property? Do you think it'd be common enough that its warranted over just using the Previous utility?

huntabyte commented 2 weeks ago

Why the previous property? Do you think it'd be common enough that its warranted over just using the Previous utility?

After thinking about it more, you're right. I think it might just create bloat that isn't always necessary, so it's best to leverage other utilities if necessary, as in the example. Disregard that one!

michal-weglarz commented 1 day ago

I've noticed an issue with updating the online field when disabling the network manually (e.g., turning off Wi-Fi on my laptop) rather than throttling the network in a browser. This can result in the value being stale and not reflecting the actual current network state. Interestingly, this problem occurs only in Chrome, not in Firefox or Safari.

We could use something like this to check again if the value is up to date:

    #updateStatus = () => {
        if (!this.#navigator) return;
        this.#online = this.#navigator.onLine;
        this.#updatedAt = new Date();

        setTimeout(() => {
            if (!this.#navigator) return;
            const newOnlineStatus = this.#navigator.onLine;
            if (this.#online !== newOnlineStatus) {
                this.#online = newOnlineStatus;
                                this.#updatedAt = new Date();
            }
        }, 100);
    };

From my manual testing, this approach indeed mitigates the problem described above. However, it seems a little brittle. Would you be fine with adding it to the code, or do you see any other potential fix?

EDIT. What would also work in this case is this:

    constructor() {
        onMount(() => {
            this.#updateStatus();

            useEventListener(window, "online", this.#updateStatus, { passive: true });
            useEventListener(window, "offline", this.#updateStatus, { passive: true });

            if (this.#connection) {
                useEventListener(this.#connection, "change", this.#updateStatus, { passive: true });
            }
        });
    }

This would probably be a better solution, but the handling of updatedAt should be reworked as well. Otherwise, based on the example included in this PR, this would result in a notification like this: '(...). Catch up with what you missed during your 0 seconds offline.' The 0 occurs because the callback updating updatedAt is called too frequently.

React-hookz/web solves this by only storing the timestamp of the moment when the status changes to online/offline. This approach doesn't include timestamps of any other updates.