fix(useOrientation): correct the inverted isBrowser guards in lockOrientation/unlockOrientation - #215
Merged
childrentime merged 1 commit intoJul 30, 2026
Conversation
…entation/unlockOrientation
Owner
|
Thanks @ostapondo! Great catch on the inverted guards — both call paths were broken and nothing in the suite noticed. Really appreciate that the tests were written to fail on main first, and the |
10 tasks
childrentime
added a commit
that referenced
this pull request
Jul 31, 2026
Fixes - useOrientation: lockOrientation/unlockOrientation had inverted isBrowser guards — they early-returned in the browser and only ran during SSR, making both no-ops where they mattered. Closes #215. - useInterval: with controls: true, an interval started through resume() kept firing after unmount, and calling resume() twice leaked a timer neither pause() nor unmount could reach. Closes #212. - useMicrophone: level stayed frozen at the last reading after stop(); it now resets to 0. Closes #213. - useElementByPoint: multiple mode re-rendered on every rAF frame because elementsFromPoint allocates a fresh array; the hit list is now compared element-by-element and kept stable when unchanged. Closes #214. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Follow-up to the note in #211: the
isBrowserguards inuseOrientation'slockOrientation/unlockOrientationare inverted:So both functions no-op in every browser —
screen.orientation.lock()/unlock()are never called — and fall through on the server, where the barewindowreference on the next line throwsReferenceError: window is not defined. Neither path had test coverage.Fix:
if (!isBrowser) return. The unsupported-browser rejection and thepromise passthrough from
lock()are unchanged — they were just unreachablebefore.
One test-infra change:
jest-setup.tstouchedwindowunconditionally, so a@jest-environment nodesuite couldn't run at all. The setup is now guardedwith
typeof window !== 'undefined'; jsdom suites are unaffected.Type of Change
Checklist
No doc change — the docs already describe the fixed behaviour.
Five tests added, four of them fail on
main:lockOrientationcallsscreen.orientation.lock(type)and returns its promiseunlockOrientationcallsscreen.orientation.unlock()lockOrientationrejects withNot supportedwhenscreen.orientationis absent@jest-environment node,renderToString) — fails onmainwith theReferenceErrorThe fifth (
unlockOrientationdoesn't throw withoutscreen.orientation) alsopasses on
main; it covers the support branch this fix makes reachable.Full suite passes (312 tests). Also verified against the built
dist/index.mjsin plain Node: render +
lock()/unlock()are no-ops, no throw.Not covered: browsers where
screen.orientationexists butlockdoesn't(iOS Safari) still throw a sync
TypeErrorrather than reject — same supportcheck as before.
#211 touches the same file, so whichever lands second may need a trivial rebase.