fix: correctly and fully SSR on deferred fragments#28
Conversation
🦋 Changeset detectedLatest commit: 30c3892 The changes in this PR will be included in the next version bump. Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Deploying with
|
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! View logs |
solid-relay | b4b5dfa | Visit preview | May 29 2025, 10:57 AM |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hello @XiNiHa, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!
Summary of Changes
Hello! Gemini or gemini-code-assist here, providing a summary of this pull request.
This PR addresses a server-side rendering (SSR) issue specifically related to deferred fragments in solid-relay. The core problem identified is that Solid's createResource mechanism, when used in this context, caches the parameters provided to its source function. This caching behavior, particularly during SSR, can lead to stale closures, meaning the resource might hold onto outdated references to reactive primitives or context values when the fragment key changes.
To fix this, the PR introduces a new approach: caching the Solid store instances themselves. These stores, which hold the fragment or query data, are now keyed by the createResource instance they are associated with, using a WeakMap stored in the Relay environment context. This ensures that when a resource re-runs (e.g., due to a key change), it either retrieves the correct, existing store instance or creates a new one, preventing stale data from being displayed or used during SSR and subsequent hydration. Additionally, the implementation avoids relying on reactive primitives specifically on the server where their behavior differs. The PR also refactors the data access pattern by replacing the custom DataProxy utility with a new DataStore utility that incorporates this caching logic and streamlines how data, loading state, and errors are exposed.
Highlights
- Fix SSR for Deferred Fragments: The primary goal is to resolve issues where deferred fragments were not correctly or fully rendering on the server due to how Solid's
createResourceinteracted with reactive data sources. - Address Stale Closures: The fix targets a specific problem where
createResourcecaching itssourceparameters leads to stale closures, causing incorrect data handling when fragment keys change, particularly during SSR. - Introduce DataStore Utility: A new utility,
DataStore, is introduced to manage the state (data, error, pending) and provide a consistent interface for accessing data from fragments and queries, replacing the previousDataProxy. - Cache Store Instances: A
WeakMapis added to the Relay environment context to cache and reuse Solid store instances based on their associatedcreateResourceinstance. This ensures updates are applied to the correct store, preventing stale data. - Refactor Primitives: The core primitives (
createFragment,createLazyLoadQuery,createPaginationFragment,createPreloadedQuery) are updated to use the newDataStoreutility and leverage the store caching mechanism.
Changelog
Click here to see the changelog
- .changeset/fast-plants-dress.md
- Add new changeset file for a minor fix.
- src/RelayEnvironment.ts
- Import
Resourcetype from 'solid-js' (line 5). - Add
dataStoresWeakMap to theRelayContexttype (line 20). - Initialize
dataStoresWeakMap in theRelayEnvironmentProvidercontext value (line 28). - Add
useDataStoreshook to access thedataStoresWeakMap from context (lines 52-57).
- Import
- src/primitives/createFragment.ts
- Update imports to include
Subscription,createSignal,untrack,SetStoreFunction,isServer,DataStore,createDataStoreand remove unused imports (createComputed,createMemo,onCleanup,DataProxy,makeDataProxy) (lines 5, 8-11). - Change return types of
createFragmentandcreateFragmentInternalfromDataProxytoDataStore(lines 36, 40, 44, 54). - Move and define
initialResultearlier (lines 57-61). - Define
resultUpdateObserverobject to handle subscriptionnextevents (lines 66-89). - Add
subscriptionsignal to manage the fragment subscription (line 90). - Add
setResultQueueand initially setsetResultto queue updates before the store is created (lines 92-97). - Refactor
createResourcesource function to untrack subscription cleanup and reset state before evaluating the key (lines 101-110). - Refactor
createResourceasync fetcher function to create the fragment source inside the async block, subscribe using theresultUpdateObserver, resolve/reject based on state, and unsubscribe on the server (lines 113-141). - Add
onHydratedoption tocreateResourceto re-subscribe after hydration (lines 144-151). - Remove the old
createStoreandcreateComputedlogic for managing the store and subscription (lines 98-141 in old code). - Use the new
createDataStoreutility to create the store, passing the resource and initial value (lines 155-158). - Process any queued
setResultcalls and reassignsetResultto the actual store setter (lines 159-162). - Return the
DataStoreproxy fromcreateDataStore(line 164).
- Update imports to include
- src/primitives/createLazyLoadQuery.ts
- Update imports to include
DataStore,createDataStoreand remove unused imports (createSignal,createStore,makeDataProxy) (lines 23, 30). - Change return types of
createLazyLoadQueryandcreateLazyLoadQueryInternalfromDataProxytoDataStore(lines 57, 87). - Remove the
serverDatasignal (line 90). - Change the type of
entryincreateResourcefetcher fromfalsetonull(line 183). - Remove the
updateDatahelper function (lines 230-234 in old code). - Use the new
createDataStoreutility to create the store, passing the initial value and resource accessor (lines 225-228). - Update the subscription handler logic to directly use
setResultwithreconcilefor data updates (lines 247-250). - Update the subscription handler logic to use
setResultfor error state (line 252). - Return the
DataStoreproxy directly (line 264).
- Update imports to include
- src/primitives/createPaginationFragment.ts
- Change type
DataProxytoDataStoreinCreatePaginationFragmentReturn(line 46). - Change type
DataProxytoDataStorefor thefragmentDataparameter increateLoadMore(line 172).
- Change type
- src/primitives/createPreloadedQuery.ts
- Change return type of
createPreloadedQueryfromDataProxytoDataStore(line 21).
- Change return type of
- src/primitives/createRefetchableFragment.ts
- Import
isServerfrom 'solid-js/web' (line 33). - Change type
DataProxytoDataStorein return types ofcreateRefetchableFragment(lines 90, 98, 108) andcreateRefetchableFragmentInternal(line 126). - Update the call to
createFragmentInternalto useparentFragmentRefon the server andfragmentRefon the client (line 273).
- Import
- src/queryCache.ts
- Import
Resourcetype from 'solid-js' (line 2). - Update
QueryCacheEntrytype: changeresourcetype from() => voidtoResource<unknown>(line 6) and replacefalsewithnull(line 9).
- Import
- src/utils/dataProxy.ts
- Delete the
dataProxy.tsfile.
- Delete the
- src/utils/dataStore.ts
- Add new file
dataStore.ts(lines 1-89). - Define the
DataStoretype (lines 6-24). - Export
createDataStorefunction which creates and returns aDataStoreproxy and its setter (lines 26-62). - Implement
createStableStorefunction which uses aWeakMapfrom context to cache and retrieve store instances based on the associated SolidResource(lines 64-79). - Implement
extractResourcehelper function (lines 81-89).
- Add new file
Using Gemini Code Assist
The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.
Invoking Gemini
You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.
| Feature | Command | Description |
|---|---|---|
| Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
| Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
| Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
| Help | /gemini help |
Displays a list of available commands. |
Customization
To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.
Limitations & Feedback
Gemini Code Assist is currently in preview and may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments to provide feedback.
You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.
Server renders fast,
Then client hydrates with care,
Data flows just right.
Footnotes
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
commit: |
There was a problem hiding this comment.
Code Review
This pull request introduces a significant and well-thought-out fix for server-side rendering (SSR) issues with deferred fragments, specifically addressing stale closure problems with Solid's createResource. The core idea of caching data store instances keyed by resource references and avoiding reactive primitives on the server where problematic is sound and effectively implemented.
The introduction of DataStore and createDataStore, along with the WeakMap-based caching in RelayEnvironmentProvider, forms a robust foundation for this fix. The changes in createFragment to manage its subscription lifecycle directly (especially unsubscribing on the server) and in createRefetchableFragment to use non-reactive refs on the server are key enablers.
Overall, this is a high-quality contribution that improves the reliability of SSR with solid-relay. The proactive approach to fixing this within the library, rather than solely relying on upstream changes in Solid, is commendable as it provides broader compatibility.
Summary of Findings
- Complexity in
createFragmentstate initialization: ThesetResultQueuemechanism increateFragment.tsintroduces some complexity to manage state updates before the final store setter is available. While it appears to solve a potential timing issue, further clarification on its necessity and design trade-offs would be beneficial. - SSR Robustness: The changes significantly improve SSR robustness by implementing a caching layer for data stores and carefully managing reactive primitives and subscriptions on the server, particularly in
createFragmentandcreateRefetchableFragment. - Refactoring from
DataProxytoDataStore: The replacement ofDataProxy/makeDataProxywithDataStore/createDataStoreis a key architectural change that centralizes the new caching logic and improves how data is handled by the primitives.
Merge Readiness
The pull request is in good shape and addresses a critical SSR issue effectively. The core changes are well-implemented.
Before merging, it would be beneficial to discuss the setResultQueue pattern in createFragment.ts (as noted in the review comment) to ensure full clarity on its design and necessity. Addressing this point would enhance the overall understanding and maintainability of this important primitive.
I am not authorized to approve pull requests, so please ensure further review and approval from authorized maintainers after considering the feedback.
| const setResultQueue: unknown[][] = []; | ||
| let setResult: SetStoreFunction<FragmentResult<TKey[" $data"]>> = ( | ||
| ...args: unknown[] | ||
| ) => { | ||
| setResultQueue.push(args); | ||
| }; |
There was a problem hiding this comment.
The introduction of setResultQueue and initializing setResult to a queuing function is an interesting pattern. This presumably handles updates that might occur (e.g., from the resource's source function or early parts of the fetcher) before createDataStore has fully initialized and returned the actual setStore function that setResult is later reassigned to.
Could you elaborate a bit on the necessity of this queuing mechanism? Was it primarily to address specific timing issues encountered with createResource's execution lifecycle in relation to the createDataStore (and its internal createStableStore) setup, especially when a store might be retrieved from the cache? Understanding the trade-offs or if alternative patterns were considered would be helpful for long-term maintainability.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #28 +/- ##
==========================================
+ Coverage 27.13% 27.85% +0.72%
==========================================
Files 21 21
Lines 1360 1411 +51
Branches 65 70 +5
==========================================
+ Hits 369 393 +24
- Misses 991 1018 +27 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
1 participant
This PR fixes SSR issues on deferred fragments caused by Solid caching
sourceparams ofcreateResource(), which leads to a stale closures issue.The solution is not to make the closures stale, by:
This fix wouldn't have been necessary if solidjs/solid#2497 had been merged, but it'd be beneficial to also fix this on our side, as it'll allow the fix to work regardless of the Solid version used.