Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions .changeset/scroll-lock-swizzle-shared.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
'@astryxdesign/core': patch
---

[fix] Two loads of Core's scroll-lock modules on one page now share one page scroll lock: closing the overlay that opened first no longer unlocks the page while the other overlay is still open. Text-selection locks, which a swizzled copy carries, are shared the same way, so a copy's release no longer restores a selection lock the package still holds. No API changes.

@cixzhang
7 changes: 7 additions & 0 deletions .changeset/shared-realm-helpers.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
'@astryxdesign/core': patch
---

[chore] Internal: Core's shared-realm module gains helpers for state that the package and a swizzled copy share (state, maps, stores, brands and sequences). No API or behavior changes.

@cixzhang
29 changes: 16 additions & 13 deletions docs/specs/AST-110-swizzle-output/spec.md

Large diffs are not rendered by default.

20 changes: 20 additions & 0 deletions packages/core/src/Layer/sharedContexts.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -154,6 +154,26 @@ describe('two Reacts in one process', () => {
expect(other).not.toBe(context);
expect(onOtherReact.createSharedContext(name, 1, 'default')).toBe(other);
});

it('gives each React its own React-bound value, and one per React', async () => {
const otherReact = secondReact();
const onThisReact = await freshInstance(
async () => import('../utils/sharedContext'),
);
const copyOnThisReact = await freshInstance(
async () => import('../utils/sharedContext'),
);
const onOtherReact = await sharedContextOn(otherReact);
const value = onThisReact.sharedForReact('test/two-reacts', 1, () => ({}));
expect(
copyOnThisReact.sharedForReact('test/two-reacts', 1, () => ({})),
).toBe(value);
const other = onOtherReact.sharedForReact('test/two-reacts', 1, () => ({}));
expect(other).not.toBe(value);
expect(onOtherReact.sharedForReact('test/two-reacts', 1, () => ({}))).toBe(
other,
);
});
});

describe('every Core context is registered under a versioned key', () => {
Expand Down
7 changes: 6 additions & 1 deletion packages/core/src/gestures/pageHold.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
import {subscribeInput} from '../utils/inputStream';
import {isCoarsePointer, watchCoarsePointer} from '../utils/coarsePointer';
import {gestureHosts} from './hostRegistry';
import {sharedWeakMap} from '../utils/sharedRealm';

/**
* Decides, once per one-finger touch, whether its pan would scroll the page:
Expand Down Expand Up @@ -71,7 +72,11 @@ interface PageHolds {
stopWatchingPointer: (() => void) | null;
}

const holdsByDocument = new WeakMap<Document, PageHolds>();
// Every copy of Core holds the page through one entry per document.
const holdsByDocument = sharedWeakMap<Document, PageHolds>(
'gestures/page-hold/holds-by-document',
1,
);

function fingersOf(event: TouchEvent): Touch[] {
return event.touches == null ? [] : Array.from(event.touches);
Expand Down
62 changes: 34 additions & 28 deletions packages/core/src/hooks/rootScrollStyles.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,8 @@
* - /packages/core/src/hooks/useScrollLock.ts
*/

import {sharedState} from '../utils/sharedRealm';

/** The root scroll styles a caller can hold. */
export interface RootScrollStyle {
/** `hidden` or `clip` stops wheel, keyboard and scrollbar scrolling of the page. */
Expand All @@ -39,12 +41,6 @@ interface Claim {
readonly value: string;
}

const claims: Record<RootScrollProperty, Claim[]> = {
overflow: [],
overscrollBehaviorY: [],
scrollbarGutter: [],
};

type SavedStyle =
| 'overflow'
| 'overflowX'
Expand Down Expand Up @@ -72,9 +68,22 @@ interface HeldTarget {
written: string;
}

const held = new Map<RootScrollProperty, HeldTarget[]>();

let nextClaimId = 0;
// Every copy of Core uses this one state, so a swizzled overlay and the
// package's stack their claims and locks on the same page.
Comment on lines +71 to +72

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Non-blocking: a swizzled overlay already reaches this module through the package's useScrollLock, so this is not the reason for the shared state. Shortest rewrite: "Every copy of Core uses this one state, so locks from two copies stack on one page."

const state = sharedState('hooks/root-scroll-styles/state', 1, () => {
const claims: Record<RootScrollProperty, Claim[]> = {
overflow: [],
overscrollBehaviorY: [],
scrollbarGutter: [],
};
return {
claims,
held: new Map<RootScrollProperty, HeldTarget[]>(),
nextClaimId: 0,
lockCount: 0,
lockListeners: new Set<(isActive: boolean) => void>(),
};
});

/**
* The element whose `overflow` the viewport uses. That is the root, unless the
Expand Down Expand Up @@ -121,11 +130,11 @@ function claimTargets(
}

function applyTopClaim(property: RootScrollProperty): void {
const targets = held.get(property);
const targets = state.held.get(property);
if (targets == null) {
return;
}
const stack = claims[property];
const stack = state.claims[property];
const top = stack[stack.length - 1];
for (const target of targets) {
const {style} = target.element;
Expand All @@ -147,7 +156,7 @@ function applyTopClaim(property: RootScrollProperty): void {
}
}
if (top == null) {
held.delete(property);
state.held.delete(property);
}
}

Expand All @@ -169,16 +178,16 @@ export function claimRootScrollStyle(claim: RootScrollStyle): () => void {
if (typeof document === 'undefined') {
return () => {};
}
nextClaimId += 1;
const id = nextClaimId;
state.nextClaimId += 1;
const id = state.nextClaimId;
const claimed: RootScrollProperty[] = [];
for (const property of PROPERTIES) {
const value = claim[property];
if (value == null) {
continue;
}
if (claims[property].length === 0) {
held.set(
if (state.claims[property].length === 0) {
state.held.set(
property,
claimTargets(property, document).map(element => {
const pageValues: Partial<Record<SavedStyle, string>> = {};
Expand All @@ -189,7 +198,7 @@ export function claimRootScrollStyle(claim: RootScrollStyle): () => void {
}),
);
}
claims[property].push({id, value});
state.claims[property].push({id, value});
claimed.push(property);
applyTopClaim(property);
}
Expand All @@ -201,7 +210,7 @@ export function claimRootScrollStyle(claim: RootScrollStyle): () => void {
}
released = true;
for (const property of claimed) {
const stack = claims[property];
const stack = state.claims[property];
const index = stack.findIndex(entry => entry.id === id);
if (index !== -1) {
stack.splice(index, 1);
Expand All @@ -211,12 +220,9 @@ export function claimRootScrollStyle(claim: RootScrollStyle): () => void {
};
}

let lockCount = 0;
const lockListeners = new Set<(isActive: boolean) => void>();

/** Whether a scroll lock (useScrollLock) currently holds the page. */
export function isScrollLockActive(): boolean {
return lockCount > 0;
return state.lockCount > 0;
}

/**
Expand All @@ -227,21 +233,21 @@ export function isScrollLockActive(): boolean {
export function subscribeScrollLock(
listener: (isActive: boolean) => void,
): () => void {
lockListeners.add(listener);
state.lockListeners.add(listener);
return () => {
lockListeners.delete(listener);
state.lockListeners.delete(listener);
};
}

/** For scroll locks: one lock taken (`1`) or released (`-1`). */
export function reportScrollLock(delta: 1 | -1): void {
const wasActive = lockCount > 0;
lockCount = Math.max(0, lockCount + delta);
const isActive = lockCount > 0;
const wasActive = state.lockCount > 0;
state.lockCount = Math.max(0, state.lockCount + delta);
const isActive = state.lockCount > 0;
if (isActive === wasActive) {
return;
}
for (const listener of [...lockListeners]) {
for (const listener of [...state.lockListeners]) {
listener(isActive);
}
}
65 changes: 65 additions & 0 deletions packages/core/src/hooks/useScrollLock.shared.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
// Copyright (c) Meta Platforms, Inc. and affiliates.

/**
* @file useScrollLock.shared.test.ts
* @input Two loads of useScrollLock and its modules, as the package and a
* swizzled copy each carry one
* @output Proves the page stays locked while a lock from either load holds,
* and comes back to its own style after the last release
* @position Shared-state tests for useScrollLock.ts and rootScrollStyles.ts
* (spec:AST-110 FR3)
*/

import {cleanup, renderHook} from '@testing-library/react';
import {afterEach, describe, expect, it, vi} from 'vitest';
import type * as RootStylesModule from './rootScrollStyles';
import type * as ScrollLockModule from './useScrollLock';

/** One load of the scroll-lock modules, as the package or a copy has. */
async function load(): Promise<{
useScrollLock: typeof ScrollLockModule.useScrollLock;
isScrollLockActive: typeof RootStylesModule.isScrollLockActive;
}> {
vi.resetModules();
const {useScrollLock} = await import('./useScrollLock');
const {isScrollLockActive} = await import('./rootScrollStyles');
return {useScrollLock, isScrollLockActive};
}

afterEach(async () => {
cleanup();
(await import('../utils/sharedRealm')).resetSharedStateForTest('hooks/');
vi.resetModules();
document.documentElement.style.overflow = '';
});

describe('a scroll lock shared by the package and a swizzled copy', () => {
it('keeps the page locked while the other load still holds a lock', async () => {
const pkg = await load();
const copy = await load();
const pkgLock = renderHook(() => pkg.useScrollLock(true));
const copyLock = renderHook(() => copy.useScrollLock(true));
expect(document.documentElement.style.overflow).toBe('hidden');

pkgLock.unmount();
// The copy's overlay is still open, so the page must stay locked.
expect(document.documentElement.style.overflow).toBe('hidden');
expect(pkg.isScrollLockActive()).toBe(true);

copyLock.unmount();
expect(document.documentElement.style.overflow).toBe('');
expect(copy.isScrollLockActive()).toBe(false);
});

it("restores the page's own overflow after the last release, in either order", async () => {
document.documentElement.style.overflow = 'auto';
const pkg = await load();
const copy = await load();
const copyLock = renderHook(() => copy.useScrollLock(true));
const pkgLock = renderHook(() => pkg.useScrollLock(true));
copyLock.unmount();
expect(document.documentElement.style.overflow).toBe('hidden');
pkgLock.unmount();
expect(document.documentElement.style.overflow).toBe('auto');
});
});
29 changes: 17 additions & 12 deletions packages/core/src/hooks/useScrollLock.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,15 +23,20 @@ import {claimRootScrollStyle, reportScrollLock} from './rootScrollStyles';
import {holdPageScroll} from '../gestures/pageHold';
import {isPageZoomedIn, isPanAbsorbedBeforePage} from './scrollLockTouchGuard';
import {holdScrollbarGutter, type ScrollbarGutterHold} from './scrollbarGutter';
import {sharedState} from '../utils/sharedRealm';

interface ActiveLock {
releaseRootStyle: () => void;
releasePageHold: () => void;
gutter: ScrollbarGutterHold;
}

let lockCount = 0;
let activeLock: ActiveLock | null = null;
// Every copy of Core counts holders here, so the page stays locked until the
// last holder in any copy releases.
const lock = sharedState('hooks/use-scroll-lock/lock', 1, () => ({
count: 0,
active: null as ActiveLock | null,
}));

/**
* Locks page scroll while `isLocked` is true.
Expand All @@ -57,7 +62,7 @@ export function useScrollLock(isLocked: boolean): void {
return;
}

if (lockCount === 0) {
if (lock.count === 0) {
// The hold measures before the lock writes a style, so the lock forces no
// layout of its own. `settle` writes the gutter with the styles below.
const gutter = holdScrollbarGutter(document.documentElement);
Expand All @@ -78,25 +83,25 @@ export function useScrollLock(isLocked: boolean): void {
!isPageZoomedIn(document) && !isPanAbsorbedBeforePage(origin, dx, dy),
);
gutter.settle();
activeLock = {releaseRootStyle, releasePageHold, gutter};
lock.active = {releaseRootStyle, releasePageHold, gutter};
}

lockCount += 1;
lock.count += 1;
reportScrollLock(1);

return () => {
lockCount -= 1;
lock.count -= 1;
reportScrollLock(-1);

if (lockCount !== 0 || activeLock == null) {
if (lock.count !== 0 || lock.active == null) {
return;
}

const lock = activeLock;
activeLock = null;
lock.releasePageHold();
lock.releaseRootStyle();
lock.gutter.release();
const active = lock.active;
lock.active = null;
active.releasePageHold();
active.releaseRootStyle();
active.gutter.release();
};
}, [isLocked]);
}
11 changes: 8 additions & 3 deletions packages/core/src/utils/selectionLock.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ import {
import {subscribeInput} from './inputStream';
import {isIosWebKit} from './touchQueries';
import {isElement, isNode} from './nodeType';
import {sharedWeakMap} from './sharedRealm';

/** What a held finger must not start on a control or a cell. */
const SELECTION_LOCK = [
Expand Down Expand Up @@ -66,14 +67,18 @@ export function locksSelectionFor(pointerType: string): boolean {
return pointerType !== 'mouse';
}

/** Locked elements, how many contacts hold each, and what to restore. */
const locks = new WeakMap<
/**
* Locked elements, how many contacts hold each, and what to restore. Every
* copy of Core counts here, so the first lock's saved styles come back only
* after the last release in any copy.
*/
const locks = sharedWeakMap<
Element,
{
count: number;
saved: ReadonlyArray<{name: string; value: string; priority: string}>;
}
>();
>('utils/selection-lock/locks', 1);

/**
* Refuse a selection and the touch callout on `elements` (and on a non-text
Expand Down
Loading
Loading