Skip to content

upgrade: devices and sensors package upgrade for Solid 2.0#905

Open
davedbase wants to merge 11 commits into
solidjs-community:nextfrom
davedbase:update/v2/devices
Open

upgrade: devices and sensors package upgrade for Solid 2.0#905
davedbase wants to merge 11 commits into
solidjs-community:nextfrom
davedbase:update/v2/devices

Conversation

@davedbase
Copy link
Copy Markdown
Member

@davedbase davedbase commented May 19, 2026

PR separates devices into devices and sensors. Adds new sensor capabilities by expanding on gyroscope and basic accelerometer primitives that already existed.

Updated: @solid-primitives/devices v2.0.0

Breaking changes:

  • createAccelerometer and createGyroscope are removed — use @solid-primitives/sensors instead
  • Requires Solid.js ^2.0.0-beta.13

Internals:

  • All createEffect calls converted to Solid 2.0 split (compute, apply) form
  • createMemo calls updated (second arg was initialValue in 1.x, now options)
  • isServer import moved from solid-js/web to @solidjs/web
  • createStore import moved from solid-js/store to solid-js

--

New package: @solid-primitives/sensors v0.1.0

Each sensor follows the Solid Primitives make* / create* convention:

  • make* — attaches listeners and returns a cleanup function; no Solid lifecycle dependency
  • create* — wraps in signals, registers cleanup via onCleanup, SSR-safe

Accelerometer (DeviceMotionEvent)

  • makeAccelerometer(onChange, { includeGravity?, interval? }) — throttled, leading-edge
  • createAccelerometer(includeGravity?, interval?)Accessor<AccelerometerReading | undefined>

Gyroscope (DeviceOrientationEvent)

  • makeGyroscope(onChange, { interval? }) — null orientation values coerced to 0
  • createGyroscope(interval?) → reactive { alpha, beta, gamma } getter object

Generic Sensor API (Chromium; covers LinearAccelerationSensor, GravitySensor, AbsoluteOrientationSensor, etc.)

  • makeSensor(SensorClass, onChange, options?)VoidFunction | null
  • createSensor(SensorClass, options?)Accessor<T | undefined> — uses { equals: false } so every reading fires the effect even when the sensor object reference is unchanged

Compass (window.Magnetometer via Generic Sensor API)

  • makeCompass(onChange, { frequency?, referenceFrame? })VoidFunction | null
  • createCompass(options?) → reactive { x, y, z } getter object (µT, null coerced to 0)

Battery (navigator.getBattery())

  • makeBattery(onChange) — async init, synchronous cleanup; safe to pass directly to onCleanup; pre-dispose race handled
  • createBattery()Accessor<BatteryReading | undefined> — starts undefined until the API resolves

Source is split into one file per sensor (accelerometer.ts, gyroscope.ts, sensor.ts, compass.ts, battery.ts) with index.ts re-exporting all of them

Summary by CodeRabbit

  • New Features

    • New @solid-primitives/sensors package providing device motion, orientation, compass, and battery sensor support through reactive APIs.
  • Breaking Changes

    • @solid-primitives/devices v2.0: now focuses on media device enumeration; accelerometer and gyroscope features moved to sensors package.
    • Updated for Solid.js v2.0.0-beta.13 compatibility.
  • Documentation

    • Updated package documentation to reflect API changes and new sensor capabilities.

Review Change Stack

@davedbase davedbase added this to the Solid 2.0 Migration milestone May 19, 2026
@changeset-bot
Copy link
Copy Markdown

changeset-bot Bot commented May 19, 2026

🦋 Changeset detected

Latest commit: 0e03082

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 2 packages
Name Type
@solid-primitives/devices Major
@solid-primitives/sensors Minor

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

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 24, 2026

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: a43198cc-9d2e-46c4-8051-ef97ab4df119

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 7

🧹 Nitpick comments (1)
packages/sensors/src/sensor.ts (1)

43-49: ⚡ Quick win

Downgrade: sensor.start() permission failures are handled via the sensor error event, not a synchronous throw; current “null only on constructor failure” contract matches docs.

  • The JSDoc/README promise null only when the constructor throws; they don’t claim start() exceptions are part of that fallback.
  • Generic Sensor API runtime permission denials during start() are typically reported asynchronously via the sensor’s error event (e.g., NotAllowedError), so missing a try/catch around start() shouldn’t break the stated fallback behavior.
  • Optional hardening: if start() can ever throw synchronously for other reasons, wrapping it and reusing the existing listener cleanup would improve resilience.
🤖 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/sensors/src/sensor.ts` around lines 43 - 49, The current code relies
on the sensor "error" event for permission failures but does not guard against
the (rare) case that sensor.start() may throw synchronously; wrap the call to
sensor.start() in a try/catch so that if start() throws you still run the same
cleanup logic (removeEventListener(handleReading) and sensor.stop()) and
optionally rethrow or call onChange(null) as your fallback contract requires;
keep the existing listener setup/teardown (handleReading,
sensor.addEventListener, sensor.removeEventListener, sensor.stop) and ensure the
catch block reuses the same cleanup code to avoid leaking listeners or running a
started sensor after an error.
🤖 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.

Inline comments:
In @.changeset/sensors-new-package.md:
- Around line 5-10: The changeset summary only lists accelerometer/gyroscope
functions; update the summary to enumerate the full exported API by adding the
missing symbols: makeSensor, createSensor, makeCompass, createCompass,
makeBattery, createBattery (in addition to makeAccelerometer/createAccelerometer
and makeGyroscope/createGyroscope), and optionally note their brief behavior
(e.g., make* raw listener returning cleanup, create* reactive/store accessors)
so release notes fully reflect the package exports.

In `@packages/devices/package.json`:
- Around line 55-61: The package declares `@solidjs/web` only as a devDependency
but imports isServer from it in packages/devices/src/index.ts and the changeset
states it must be a peer dependency; update packages/devices/package.json so
that `@solidjs/web` is added to "peerDependencies" (use the same version spec as
the existing devDependency, e.g. "2.0.0-beta.13") and keep or remove the
devDependency as desired, ensuring the peerDependencies section for the
`@solid-primitives/devices` package includes "`@solidjs/web`" alongside "solid-js".

In `@packages/devices/README.md`:
- Around line 17-21: The installation code fence in packages/devices/README.md
is missing a language tag causing MD040 lint failures; update the opening fence
for the block containing "npm install `@solid-primitives/devices`" and "pnpm add
`@solid-primitives/devices`" to include a language tag (e.g., change ``` to
```bash) so the fenced code block is properly recognized by the markdown linter.

In `@packages/sensors/package.json`:
- Line 4: Update the package.json "description" value to accurately reflect the
current feature set: replace the existing text that only mentions "accelerometer
and gyroscope" with a concise summary that includes generic sensor, compass, and
battery primitives (e.g., mention "accelerometer, gyroscope, generic Sensor API,
compass, and battery" or similar) so the package metadata aligns with the code
in packages/sensors and improves discoverability.

In `@packages/sensors/README.md`:
- Around line 15-19: Update the installation code fence in the README so the
triple-backtick block that contains "npm install `@solid-primitives/sensors`" /
"pnpm add `@solid-primitives/sensors`" is a typed fence (e.g. change ``` to
```bash) to satisfy markdownlint and ensure proper syntax highlighting for that
installation snippet.

In `@packages/sensors/test/index.test.ts`:
- Around line 390-393: The test currently only checks that start() was called
but doesn't verify the constructor received the expected options; update the
constructor in the test helper (where constructor(_opts?: any) { super();
lastCompassInstance = this; }) to capture and store the incoming args (e.g. set
lastCompassInstance.constructorArgs = _opts or set a separate lastCompassArgs
variable) and then update the test titled “constructor options are passed” to
assert those stored constructor arguments equal the expected options in addition
to keeping the existing assertion that start() was called; reference the test
helper's constructor, lastCompassInstance, and the test that asserts start().
- Around line 650-673: The Promise executor is marked async which can mask
errors; remove async from new Promise<void>(async (resolve, reject) => { ... })
and instead explicitly schedule microtasks (e.g., use
Promise.resolve().then(...) or queueMicrotask) where the code currently does
await Promise.resolve(); keep the same logic around createBattery(),
createEffect(...), mockBattery.dispatchEvent(...), and the setTimeout timeout
reject — ensure dispose() and resolve() are called from the non-async executor
so thrown errors propagate to the Promise rejection instead of being swallowed
by an async executor.

---

Nitpick comments:
In `@packages/sensors/src/sensor.ts`:
- Around line 43-49: The current code relies on the sensor "error" event for
permission failures but does not guard against the (rare) case that
sensor.start() may throw synchronously; wrap the call to sensor.start() in a
try/catch so that if start() throws you still run the same cleanup logic
(removeEventListener(handleReading) and sensor.stop()) and optionally rethrow or
call onChange(null) as your fallback contract requires; keep the existing
listener setup/teardown (handleReading, sensor.addEventListener,
sensor.removeEventListener, sensor.stop) and ensure the catch block reuses the
same cleanup code to avoid leaking listeners or running a started sensor after
an error.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 52eed979-4069-4ae7-916e-b58484ad988f

📥 Commits

Reviewing files that changed from the base of the PR and between 76d6f2e and 864e6c7.

⛔ Files ignored due to path filters (1)
  • pnpm-lock.yaml is excluded by !**/pnpm-lock.yaml
📒 Files selected for processing (19)
  • .changeset/devices-solid2-migration.md
  • .changeset/sensors-new-package.md
  • packages/devices/README.md
  • packages/devices/package.json
  • packages/devices/src/index.ts
  • packages/devices/test/index.test.ts
  • packages/devices/test/server.test.ts
  • packages/sensors/CHANGELOG.md
  • packages/sensors/README.md
  • packages/sensors/package.json
  • packages/sensors/src/accelerometer.ts
  • packages/sensors/src/battery.ts
  • packages/sensors/src/compass.ts
  • packages/sensors/src/gyroscope.ts
  • packages/sensors/src/index.ts
  • packages/sensors/src/sensor.ts
  • packages/sensors/test/index.test.ts
  • packages/sensors/test/server.test.ts
  • packages/sensors/tsconfig.json

Comment thread .changeset/sensors-new-package.md Outdated
Comment thread packages/devices/package.json Outdated
Comment thread packages/devices/README.md
Comment thread packages/sensors/package.json Outdated
Comment thread packages/sensors/README.md Outdated
Comment thread packages/sensors/test/index.test.ts
Comment thread packages/sensors/test/index.test.ts Outdated
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant