feat: `NetworkStatus` #117

michal-weglarz · 2024-07-11T10:39:18Z

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:

use-network-status-chrome-demo.mov

Demo in Safari/Firefox:

use-network-stastus-firefox-safari-demo.mov

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 · 2024-07-11T10:39:22Z

⚠️ No Changeset found

Latest commit: 6b7f63e

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 · 2024-07-11T10:40:19Z

built with Refined Cloudflare Pages Action

⚡ Cloudflare Pages Deployment

Name	Status	Preview	Last Commit
runed	✅ Ready (View Log)	Visit Preview	`14c85cb`

chore: add .idea to gitignore chore: clean up chore: clean up

Not-Jayden

Nice one!

Left some small nitpicks.

More broadly, I don't want to speak for the maintainers, but looking at the current naming/API patterns of the codebase my assumption is the use prefix is being reserved for functions that accept a callback to observe changes.

I'd probably either rework so it can be used like:

useNetworkStatus((data) => { 
  // do something with new data 
});

or keep it essentially the same as the current implementation, but change it to a NetworkStatus class (classes also seem to be the pattern for encapsulating reactive state in this repo).

const networkStatus = new NetworkStatus();

Second makes most sense to me, but you could potentially even have both if you really wanted. Hopefully maintainers can give guidance.

packages/runed/src/lib/utilities/useNetworkStatus/useNetworkStatus.svelte.ts

refactor: refactor the way event listeners are removed on unmount

huntabyte

Thanks for taking the time to contribute this! Left some notes!

packages/runed/src/lib/utilities/useNetworkStatus/useNetworkStatus.svelte.ts

Not-Jayden

Looking great! Just passing observations until maintainers can provide a proper review :)

sites/docs/content/utilities/network-status.md

packages/runed/src/lib/utilities/NetworkStatus/NetworkStatus.svelte.ts

huntabyte · 2024-07-18T01:36:38Z

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 · 2024-07-18T08:20:30Z

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

huntabyte · 2024-07-19T15:37:50Z

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!

TGlide

Awesome stuff! Some minor changes needed

packages/runed/src/lib/utilities/NetworkStatus/NetworkStatus.svelte.ts

TGlide · 2024-07-19T17:44:40Z

packages/runed/src/lib/utilities/NetworkStatus/NetworkStatus.svelte.ts

+	);
+	#online: boolean = $state(false);
+	#updatedAt: Date = $state(new Date());
+	#downlink?: NetworkInformation["downlink"] = $state();


Can't these be deriveds that depend on connection?

Actually, can't we just use gets that use connection? Why do we need these individual fields?

Can we get the updated state without these? If connection or its properties, or online/offline status changes (e.g., random internet disconnection), how will the client be notified?

connection can be a state, since all of the other ones are just set from it. then updateStatus updates connection, and that's it

I pushed the commit where I simplified the state by depending solely on the derived connection. Seems like it's working exactly the same as before so definitely a great suggestion :)

packages/runed/src/lib/utilities/NetworkStatus/NetworkStatus.svelte.ts

sites/docs/content/utilities/network-status.md

TGlide · 2024-07-19T17:47:10Z

sites/docs/content/utilities/network-status.md

+
+```ts
+const networkStatus = new NetworkStatus();
+const previousNetworkStatus = new Previous(() => ({


tbh its a bit of a shame we can't spread here 🤔

@TGlide this is literally what made me consider doing the previous for the user for this one so they wouldn't have to do line by line. The only way I see being able to spread is exposing a $derived property on the class that is basically an object with all those props networkStatus.obj or networkStatus.all but idk how it feels or if it's worth the extra bloat tbh

Hmm tbh its probably fine. Most of these properties are just being proxied within the connection property, so you could just use that

TGlide · 2024-07-19T17:48:39Z

sites/docs/src/lib/components/demos/network-status.svelte

+
+<DemoContainer>
+	{#if networkStatus.isSupported}
+		<pre>{JSON.stringify(networkStatusCombined, null, 2)}</pre>


@huntabyte is it too hard to get syntax highlighting here?

@TGlide, uhh, we could hack it a bit, but without hacks, we'd need to ship Shiki to the browser.

No need then!

michal-weglarz · 2024-08-02T14:57:37Z

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.

github-actions bot deployed to Preview July 11, 2024 10:40 View deployment

github-actions bot deployed to Preview July 11, 2024 10:42 View deployment

feat: add useNetworkStatus util

e529bf5

chore: add .idea to gitignore chore: clean up chore: clean up

michal-weglarz force-pushed the feat-add-use-network-status-util branch from e94cd51 to e529bf5 Compare July 11, 2024 10:46

github-actions bot deployed to Preview July 11, 2024 10:47 View deployment

Not-Jayden reviewed Jul 12, 2024

View reviewed changes

github-actions bot requested a deployment to Preview July 12, 2024 16:20 Abandoned

refactor: refactor the way event listeners are removed on unmount

c9f45b2

refactor: refactor the way event listeners are removed on unmount

michal-weglarz force-pushed the feat-add-use-network-status-util branch from f72ffda to c9f45b2 Compare July 12, 2024 16:22

github-actions bot deployed to Preview July 12, 2024 16:22 View deployment

huntabyte requested changes Jul 12, 2024

View reviewed changes

packages/runed/src/lib/utilities/useNetworkStatus/useNetworkStatus.svelte.ts Outdated Show resolved Hide resolved

packages/runed/src/lib/utilities/useNetworkStatus/useNetworkStatus.svelte.ts Outdated Show resolved Hide resolved

refactor: rewrite useNetworkStatus to a class

15ccf0a

github-actions bot deployed to Preview July 13, 2024 10:26 View deployment

michal-weglarz requested review from huntabyte and Not-Jayden July 16, 2024 12:42

Not-Jayden reviewed Jul 16, 2024

View reviewed changes

sites/docs/content/utilities/network-status.md Outdated Show resolved Hide resolved

Not-Jayden reviewed Jul 16, 2024

View reviewed changes

packages/runed/src/lib/utilities/NetworkStatus/NetworkStatus.svelte.ts Outdated Show resolved Hide resolved

touch ups

2d02f6f

github-actions bot deployed to Preview July 18, 2024 01:33 View deployment

more touch ups

1096b1f

github-actions bot deployed to Preview July 18, 2024 01:35 View deployment

formatting

f1e39ae

huntabyte requested a review from TGlide July 18, 2024 01:45

github-actions bot deployed to Preview July 18, 2024 01:46 View deployment

TGlide requested changes Jul 19, 2024

View reviewed changes

chore: minor fixes

c406865

github-actions bot deployed to Preview July 21, 2024 19:48 View deployment

refactor: replace addEventListener with useEventListener

14c85cb

github-actions bot deployed to Preview July 21, 2024 19:54 View deployment

refactor: simplify internal state by depending on derived connection

baaff33

michal-weglarz requested a review from TGlide August 2, 2024 15:12

huntabyte added 3 commits December 19, 2024 18:22

Merge branch 'main' into feat-add-use-network-status-util

bae76e6

fix lockfile

ffa8774

tidy up

831df2d

huntabyte changed the title ~~feat: add useNetworkStatus~~ feat: NetworkStatus Dec 19, 2024

more

6b7f63e

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: `NetworkStatus` #117

feat: `NetworkStatus` #117

michal-weglarz commented Jul 11, 2024

changeset-bot bot commented Jul 11, 2024 •

edited

Loading

github-actions bot commented Jul 11, 2024 •

edited

Loading

Not-Jayden left a comment

huntabyte left a comment

Not-Jayden left a comment

huntabyte commented Jul 18, 2024

TGlide commented Jul 18, 2024

huntabyte commented Jul 19, 2024 •

edited

Loading

TGlide left a comment

TGlide Jul 19, 2024

TGlide Jul 19, 2024

michal-weglarz Jul 22, 2024

TGlide Jul 31, 2024

michal-weglarz Aug 2, 2024

TGlide Jul 19, 2024

huntabyte Jul 27, 2024

TGlide Jul 31, 2024

TGlide Jul 19, 2024

huntabyte Jul 27, 2024

TGlide Jul 31, 2024

michal-weglarz commented Aug 2, 2024 •

edited

Loading

feat: NetworkStatus #117

Are you sure you want to change the base?

feat: NetworkStatus #117

Conversation

michal-weglarz commented Jul 11, 2024

changeset-bot bot commented Jul 11, 2024 • edited Loading

⚠️ No Changeset found

github-actions bot commented Jul 11, 2024 • edited Loading

built with Refined Cloudflare Pages Action

⚡ Cloudflare Pages Deployment

Not-Jayden left a comment

Choose a reason for hiding this comment

huntabyte left a comment

Choose a reason for hiding this comment

Not-Jayden left a comment

Choose a reason for hiding this comment

huntabyte commented Jul 18, 2024

TGlide commented Jul 18, 2024

huntabyte commented Jul 19, 2024 • edited Loading

TGlide left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

michal-weglarz commented Aug 2, 2024 • edited Loading

feat: `NetworkStatus` #117

feat: `NetworkStatus` #117

changeset-bot bot commented Jul 11, 2024 •

edited

Loading

github-actions bot commented Jul 11, 2024 •

edited

Loading

huntabyte commented Jul 19, 2024 •

edited

Loading

michal-weglarz commented Aug 2, 2024 •

edited

Loading