Skip to content

fix: no-op when removing an already-removed glass#114

Merged
ohxyz merged 1 commit into
mainfrom
fix/remove-glass-null-guard
Jun 30, 2026
Merged

fix: no-op when removing an already-removed glass#114
ohxyz merged 1 commit into
mainfrom
fix/remove-glass-null-guard

Conversation

@ohxyz

@ohxyz ohxyz commented Jun 30, 2026

Copy link
Copy Markdown
Collaborator

Problem

removeWindowlessGlass and removeDetachedGlass pass detachedGlassManager.removeDetachedGlass(id) straight into removeDetachedGlassElement, which dereferences detachedGlassEl.id (via removeGlassBackdrop).

detachedGlassManager.removeDetachedGlass(id) returns null when the id isn't found — which happens whenever the glass was already removed, e.g. closed via its built-in close action. Removing it again by a stale id then crashes:

Uncaught (in promise) TypeError: Cannot read properties of null (reading 'id')

Repro (via react-bwin)

  1. Open two windowless glasses.
  2. Close one via its close button (runs the built-in action → removeWindowlessGlass directly; react-bwin's live-id set still holds the closed id).
  3. Unmount the WindowProvider → cleanup calls removeWindowlessGlass(staleId, { animate: false }) → crash.

Fix

Guard the null return in both remove paths so removing an already-gone glass is a harmless no-op (returns Promise.resolve(null)), consistent with the manager already returning null for an unknown id.

Test plan

  • Call removeWindowlessGlass(id) twice for the same glass — second call resolves to null, no throw.
  • Same for removeDetachedGlass(id).
  • react-bwin: open two windowless glasses, close one, unmount the provider — no error, no orphaned glass.

removeWindowlessGlass and removeDetachedGlass passed the manager's return
straight into removeDetachedGlassElement, which dereferences `.id`. When the
glass was already gone (e.g. closed via its action) the manager returns null,
crashing with "Cannot read properties of null (reading 'id')". Guard the null
so a stale id is a harmless no-op.
@ohxyz ohxyz requested a review from bhjsdev as a code owner June 30, 2026 01:55
@ohxyz ohxyz merged commit 92ca458 into main Jun 30, 2026
1 check passed
@ohxyz ohxyz deleted the fix/remove-glass-null-guard branch June 30, 2026 01:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant