Skip to content

Add structuredClone fallback for older browsers - #39

Merged
mozzius merged 1 commit into
mainfrom
samuel/fix-structured-clone-fallback
Aug 28, 2026
Merged

Add structuredClone fallback for older browsers#39
mozzius merged 1 commit into
mainfrom
samuel/fix-structured-clone-fallback

Conversation

@mozzius

@mozzius mozzius commented Aug 28, 2026

Copy link
Copy Markdown
Member

Summary

  • fall back to JSON cloning when structuredClone is unavailable
  • preserve deep-clone behavior for rich-text facets
  • add a patch changeset for @bsky/sdk

Test plan

  • In a browser without structuredClone, clone rich text with a facet and confirm mutating the clone does not affect the original.

@claude

claude Bot commented Aug 28, 2026

Copy link
Copy Markdown

Claude finished @mozzius's task in 59s —— View job


Review: Add structuredClone fallback for older browsers

  • Read review guidance (.github/claude-review-prompt.md)
  • Gather context (diff, changed files)
  • Review the changes
  • Post review feedback

No blocking findings — this PR looks good to merge.

The change is correct and appropriately scoped. Notes on what I checked:

  • JSON round-trip is equivalent here. Facet is app.bsky.richtext.facet.Main, a lexicon-generated plain-object type (index + features, all JSON-serializable). It has no Date, Map, Set, undefined-valued properties, or cyclic references, so JSON.parse(JSON.stringify(...)) produces the same deep clone structuredClone would. The mutation-isolation test confirms the deep-copy behavior.
  • The undefined guard is necessary and correct. this.facets is optional (Facet[] | undefined). structuredClone(undefined) returns undefined, but the fallback's JSON.parse(JSON.stringify(undefined)) would throw (JSON.stringify(undefined) → the value undefinedJSON.parse on it is a SyntaxError). The early typeof value === 'undefined' return handles this, and the test's new RichText({ text: 'test' }).clone().facets assertion covers it.
  • Feature detection is safe. typeof structuredClone === 'function' on a possibly-undeclared global doesn't throw, and the test stubs it to undefined to exercise the fallback path.
  • Consistency: this was the only structuredClone usage in @bsky/sdk (both clone() and copyInto() now route through cloneDeep), so nothing is left on the old path.
  • Changeset: patch for @bsky/sdk is the right bump — bug fix, no public API/type change.

One minor, non-blocking thought (take it or leave it): JSON.parse(JSON.stringify(...)) silently drops any future non-serializable facet field. That's a non-issue for today's lexicon-defined Facet, but if facets ever grow richer shapes, a structural clone helper would be more future-proof. Not worth changing now.
· samuel/fix-structured-clone-fallback

@mozzius
mozzius merged commit 1a154a4 into main Aug 28, 2026
8 checks passed
@mozzius
mozzius deleted the samuel/fix-structured-clone-fallback branch August 28, 2026 14:24
@github-actions github-actions Bot mentioned this pull request Aug 28, 2026
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.

2 participants