Skip to content

fix(jsx): handle useSyncExternalStore subscription and snapshot changes - #5166

Merged
yusukebe merged 4 commits into
mainfrom
fix/use-sync-external-store
Jul 26, 2026
Merged

fix(jsx): handle useSyncExternalStore subscription and snapshot changes#5166
yusukebe merged 4 commits into
mainfrom
fix/use-sync-external-store

Conversation

@usualoma

Copy link
Copy Markdown
Member

fixes #5162

The reported issue can be addressed by making the subscription effect depend on subscribe. This PR goes slightly further and also handles changes to getSnapshot:

  • Re-subscribe when subscribe changes.
  • Keep the previous subscription active until the new subscription is installed during the effect flush.
  • Read the snapshot during render so a changed getSnapshot is reflected without waiting for a store notification.
  • Re-check the latest snapshot after subscribing to avoid missing an update between render and effect execution.
  • Add regression tests for changing subscribe and getSnapshot.

While investigating this, I also explored fixing the underlying deferred effect and cleanup behavior. Since that would affect all effect hooks and needs broader review, it is intentionally kept out of this PR; see the draft PoC in #5165.

The author should do the following, if applicable

  • Add tests
  • Run tests
  • bun run format:fix && bun run lint:fix to format the code
@github-actions

Copy link
Copy Markdown

Bundle size check

main (26d8e42) #5166 (2819497) +/-
Bundle Size (B) 19,032B 19,032B 0B
Bundle Size (KB) 18.59K 18.59K 0K

Compiler Diagnostics (tsc)

main (26d8e42) #5166 (2819497) +/-
Files 83 83 0
Lines 59,439 59,439 0
Identifiers 60,614 60,614 0
Symbols 105,939 105,939 0
Types 159,727 159,727 0
Instantiations 413,958 413,958 0
Memory used 167,608K 168,654K 1,046K
I/O read 0.01s 0.01s 0s
I/O write 0s 0s 0s
Parse time 0.35s 0.4s 0.05s
Bind time 0.16s 0.15s -0.01s
Check time 1.4s 1.36s -0.04s
Emit time 0s 0s 0s
Total time 1.91s 1.91s 0s

Compiler Diagnostics (typescript-go)

main (26d8e42) #5166 (2819497) +/-
Files 215 215 0
Lines 147,087 147,087 0
Identifiers 121,968 121,968 0
Symbols 188,307 188,307 0
Types 259,012 259,012 0
Instantiations 621,523 621,523 0
Memory used 160,754K 160,817K 63K
Memory allocs 1,776,854 1,776,623 -231
Config time 0.001s 0.001s 0s
BuildInfo read time 0s 0s 0s
Parse time 0.097s 0.09s -0.007s
Bind time 0s 0s 0s
Check time 0.455s 0.456s 0.001s
Emit time 0.005s 0.005s 0s
Changes compute time 0.024s 0.024s 0s
Total time 0.621s 0.614s -0.007s

Reported by octocov

@usualoma

Copy link
Copy Markdown
Member Author

Hi @yusukebe,
Could you please leave a review?

@github-actions

Copy link
Copy Markdown

HTTP Performance Benchmark

Framework Runtime Average Ping Query Body
hono (origin/main) bun 62,879.75 81,314.61 59,363.85 47,960.78
hono (current) bun 63,106.27 81,433.31 59,952.22 47,933.28
Change +0.36% +0.15% +0.99% -0.06%
@codecov

codecov Bot commented Jul 26, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 86.66667% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 79.09%. Comparing base (c285f9a) to head (0c7ad71).
⚠️ Report is 14 commits behind head on main.

Files with missing lines Patch % Lines
src/jsx/hooks/index.ts 86.66% 2 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##             main    #5166   +/-   ##
=======================================
  Coverage   79.08%   79.09%           
=======================================
  Files         154      154           
  Lines       10764    10782   +18     
  Branches     2254     2253    -1     
=======================================
+ Hits         8513     8528   +15     
- Misses       2251     2254    +3     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@yusukebe yusukebe left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM!

@yusukebe

Copy link
Copy Markdown
Member

@usualoma

Thank you!

@yusukebe
yusukebe merged commit 224d2f5 into main Jul 26, 2026
20 checks passed
@yusukebe
yusukebe deleted the fix/use-sync-external-store branch July 26, 2026 05:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

2 participants