upgrade: connectivity package upgrade for Solid 2.0#931
Conversation
🦋 Changeset detectedLatest commit: 86f22e2 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/connectivity/src/index.ts (1)
211-219: ⚡ Quick winInconsistent signal setter pattern.
Line 212 passes the value directly to
setOnline(state.online), while lines 213–218 wrap each value in an arrow function:setDownlink(() => state.downlink). Both patterns work (Solid accepts values or updater functions), but the inconsistency is confusing.For clarity and brevity, use the direct-value pattern consistently across all setters.
♻️ Proposed fix for consistency
makeNetworkInformation(state => { setOnline(state.online); - setDownlink(() => state.downlink); - setDownlinkMax(() => state.downlinkMax); - setEffectiveType(() => state.effectiveType); - setRtt(() => state.rtt); - setSaveData(() => state.saveData); - setType(() => state.type); + setDownlink(state.downlink); + setDownlinkMax(state.downlinkMax); + setEffectiveType(state.effectiveType); + setRtt(state.rtt); + setSaveData(state.saveData); + setType(state.type); });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/connectivity/src/index.ts` around lines 211 - 219, The setters in the makeNetworkInformation callback use mixed patterns: setOnline(state.online) vs setDownlink(() => state.downlink) etc.; make them consistent by passing the direct value for all signals (use setDownlink(state.downlink), setDownlinkMax(state.downlinkMax), setEffectiveType(state.effectiveType), setRtt(state.rtt), setSaveData(state.saveData), setType(state.type)) while leaving makeNetworkInformation and setOnline unchanged so all signals use the same direct-value update style.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@packages/connectivity/src/index.ts`:
- Around line 211-219: The setters in the makeNetworkInformation callback use
mixed patterns: setOnline(state.online) vs setDownlink(() => state.downlink)
etc.; make them consistent by passing the direct value for all signals (use
setDownlink(state.downlink), setDownlinkMax(state.downlinkMax),
setEffectiveType(state.effectiveType), setRtt(state.rtt),
setSaveData(state.saveData), setType(state.type)) while leaving
makeNetworkInformation and setOnline unchanged so all signals use the same
direct-value update style.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: b4bb0b60-17da-49a2-92f7-db8d5937a35a
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (10)
.changeset/connectivity-solid2-migration.mdpackages/connectivity/README.mdpackages/connectivity/dev/index.tsxpackages/connectivity/package.jsonpackages/connectivity/src/index.tspackages/connectivity/stories/connectivity.stories.tsxpackages/connectivity/stories/tsconfig.jsonpackages/connectivity/test/index.test.tspackages/connectivity/test/server.test.tspackages/connectivity/test/setup.ts
💤 Files with no reviewable changes (1)
- packages/connectivity/dev/index.tsx
Solid 2.0 migration**
solid-js@^2.0.0-beta.14and@solidjs/web@^2.0.0-beta.14isServerimport moved fromsolid-js/web→@solidjs/webownedWrite: trueso the event-listener setter can be called from within reactive scopes (e.g. synchronousdispatchEventin tests) without throwingSIGNAL_WRITE_IN_OWNED_SCOPEflush()after signal writes to account for Solid 2.0's async batchingNew: Network Information API primitives
makeNetworkInformation(callback)— low-level listener combiningwindowonline/offline events withnavigator.connectionchange events; fires aNetworkStatesnapshot on any changecreateNetworkInformation()→{ online, downlink, downlinkMax, effectiveType, rtt, saveData, type }— independent reactive signals for the full network state; suitable for adaptive loading (image quality, prefetch, feature flags based on connection conditions)useNetworkInformation()— singleton-root variant sharing a single listener set across all callersNetworkState,NetworkInformationReturn,EffectiveConnectionType,ConnectionTypenavigator.connectionaugmented viadeclare global(not in TypeScript 5.8's stdlib)undefinedgracefully in Firefox/Safari — no errors thrownStories
Network/Connectivity:createConnectivitySignalbadgecreateNetworkInformationpanel with adaptive quality indicatormakeNetworkInformationcallback log with timestampsdev/index.tsxexampleSummary by CodeRabbit
Release Notes
New Features
Documentation