chore: refactoring react server component client#1186
Conversation
|
@launchdarkly/js-sdk-common size report |
|
@launchdarkly/browser size report |
|
@launchdarkly/js-client-sdk-common size report |
|
@launchdarkly/js-client-sdk size report |
6e30ca5 to
562a3e6
Compare
848e4c8 to
562a3e6
Compare
|
@cursor review |
| // of meta with exclusions. | ||
| // eslint-disable-next-line no-param-reassign | ||
| opts.mangleProps = /^_([^m|_]|m[^e]|me[^t]|met[^a])/; | ||
| opts.banner = { js: '"use client";' }; |
There was a problem hiding this comment.
This is the reason that we separated the client bundle and server bundle
562a3e6 to
4bcbba2
Compare
4bcbba2 to
61658d1
Compare
There was a problem hiding this comment.
Ditching the context getter design and instead we are give the users a way to create a "scoped proxy" of the base server sdk for each request session that the LDClient is needed.
|
@cursor review |
| client.jsonVariationDetail(key, context, defaultValue), | ||
| allFlagsState: (options?: LDFlagsStateOptions) => client.allFlagsState(context, options), | ||
| }; | ||
| } |
There was a problem hiding this comment.
Internal helper createLDServerWrapper is publicly exported
Medium Severity
createLDServerWrapper is exported from LDServerSession.ts and re-exported via export * from './LDServerSession' in index.ts, making it part of the public API. However, it's only called internally by createLDServerSession and bypasses the cache() storage. If a user calls createLDServerWrapper directly, the session won't be stored in React's per-request cache, causing useLDServerSession() to return null — silently breaking the documented pattern. This function is an internal implementation detail and likely shouldn't be exported.
Additional Locations (1)
There was a problem hiding this comment.
This is by design in case developers do not want to use our cache implementation. This is the same reasoning as the ability to use named context in client components.
There was a problem hiding this comment.
I do wonder if we need some way of highlighting this as an exposing API.
Most of the time everything exposed is inside api, with the exception of like a factory in the index.
There was a problem hiding this comment.
yea I'll think about that some more... I believe we did have a few issues in the past where clients didn't know about certain APIs are available.
fc1d11f to
f2eee29
Compare
1f577ee to
ef011c4
Compare
| @@ -0,0 +1,3 @@ | |||
| # Temporary ignore to keep PRs manageable. | |||
|
|
|||
| server-only | |||
There was a problem hiding this comment.
Will commit the example application in a separate PR. Wanted to focus this one on the implementation changes
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
There are 3 total unresolved issues (including 1 from previous review).
Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.
| // cache() creates a per-request memoized store — each React render tree (request) | ||
| // gets its own isolated instance. The store is populated by createLDServerSession | ||
| // and read by useLDServerSession. | ||
| const withCache = cache(() => ({ session: null as LDServerSession | null })); |
There was a problem hiding this comment.
Server bundle uses cache unavailable in peer dependency range
Low Severity
The server bundle now imports cache from react at module scope and calls it immediately (const withCache = cache(...)). react.cache only exists in React 19+ (or React 18 canary used by Next.js). The package's peerDependencies declares react >= 18.0.0, which includes React 18 stable where cache is not exported. Anyone importing @launchdarkly/react-sdk/server with React 18 stable will get a crash at module load time since cache is undefined.
There was a problem hiding this comment.
This will need to be documented. This also should not be a problem functionally speaking because RSC were not stabilized until react 19. This means that developer should hit errors much sooner if they import from @launchdarkly/react-sdk/server when using a react 18 base.
| /** | ||
| * The LaunchDarkly server client interface for React. | ||
| * A per-request evaluation scope that binds an {@link LDServerBaseClient} to a specific | ||
| * {@link LDContext}. |
There was a problem hiding this comment.
I assume this approximately matches the broader scoped-client design?
There was a problem hiding this comment.
I think so, I made it so that it should be easily compatible/migrate-able if we decide to implement scoped client capabilities in the underlying server sdks


This PR is to finalize how the react server components can access server side LDClient. A few key points:
This address:
SDK-2026
SDK-2021
Note
Medium Risk
Moderate risk due to a breaking redesign of the server entrypoint API (removing
createReactServerClient/context-provider options and changing browser behavior from no-op to throw), which could affect existing integrations and bundling boundaries.Overview
Refactors the React SDK server entrypoint to use a per-request
LDServerSession(context bound at creation) stored in Reactcache(), exposed viacreateLDServerSession/useLDServerSession, and enforces server-only usage by throwing in browser environments rather than returning a no-op client.Removes the previous
createReactServerClient+LDContextProvider/LDReactServerOptionsAPI, introducesLDServerBaseClientas a minimal structural interface, and updates docs/examples accordingly. Adds Jest coverage for session binding/caching behavior and adjusts build tooling to emit separate client/server bundles (including a"use client"banner for the client entry) plus small eslint/gitignore tweaks for examples.Written by Cursor Bugbot for commit 80eca87. This will update automatically on new commits. Configure here.