feat(react): support lazy activity with internal plugin #617
Conversation
Deploying stackflow-demo with
|
| Latest commit: |
7726c76
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://f4d7e0d0.stackflow-demo.pages.dev |
| Branch Preview URL: | https://lazy-activity.stackflow-demo.pages.dev |
🦋 Changeset detectedLatest commit: 7726c76 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
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 |
📝 WalkthroughSummary by CodeRabbit
WalkthroughAdds a new internal React plugin that detects React.lazy and memoized activity components, pauses navigation while unresolved lazy components load, and conditionally registers the plugin for React major versions 18–19; includes a changeset for a minor @stackflow/react release. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Stackflow
participant LazyPlugin as lazyActivityPlugin
participant ReactLazy as ReactLazyComponent
User->>Stackflow: request navigation (push/replace)
Stackflow->>LazyPlugin: onBeforePush/onBeforeReplace
LazyPlugin->>ReactLazy: inspect activity component (lazy/memo)
alt component is lazy and uninitialized
LazyPlugin->>Stackflow: pause navigation
LazyPlugin->>ReactLazy: start loading (invoke promise)
ReactLazy-->>LazyPlugin: promise resolves / rejects
LazyPlugin->>Stackflow: resume navigation
else not lazy or already loaded
LazyPlugin->>Stackflow: allow navigation to continue
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. 📜 Recent review detailsConfiguration used: .coderabbit.yaml 💡 Knowledge Base configuration:
You can enable these sources in your CodeRabbit configuration. 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
Status, Documentation and Community
|
Deploying with
|
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! View logs |
stackflow-docs | 7726c76 | Commit Preview URL | Aug 20 2025, 06:45 AM |
commit: |
orionmiz
changed the title
orionmiz
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
integrations/react/src/stable/lazyActivityPlugin.ts (2)
43-46: Consider using an enum for status constantsThe status constants would be more maintainable and type-safe as an enum:
-const Uninitialized = -1; -const Pending = 0; -const Resolved = 1; -const Rejected = 2; +enum LazyStatus { + Uninitialized = -1, + Pending = 0, + Resolved = 1, + Rejected = 2, +}Then update the type definitions to use the enum:
-type UninitializedPayload<T> = { - _status: -1; +type UninitializedPayload<T> = { + _status: LazyStatus.Uninitialized;
126-133: Add error logging for failed lazy loadingWhen lazy loading fails, the navigation is resumed but the error is silently swallowed. This could make debugging difficult.
Activity._payload._result().then( () => { actions.resume(); }, - () => { + (error) => { + console.error(`Failed to load lazy activity "${actionParams.activityName}":`, error); actions.resume(); }, );
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.changeset/small-rice-listen.md(1 hunks)integrations/react/src/stable/lazyActivityPlugin.ts(1 hunks)integrations/react/src/stable/stackflow.tsx(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Workers Builds: stackflow-docs
🔇 Additional comments (4)
integrations/react/src/stable/lazyActivityPlugin.ts (1)
137-146: Well-structured plugin implementationGood use of the factory pattern and code reuse by extracting the common logic to
handleLazyActivity.integrations/react/src/stable/stackflow.tsx (2)
23-23: Import statement looks good
144-149: Strategic plugin placement ensures compatibilityPlacing the
lazyActivityPluginat the end of the plugins array is a good design choice. This ensures user plugins execute first and can potentially modify the activity map before lazy loading logic runs..changeset/small-rice-listen.md (1)
1-5: Appropriate changeset documentationThe minor version bump is correct for this new feature addition, and the description clearly communicates the change.
| // https://github.com/facebook/react/blob/v19.1.1/packages/shared/ReactSymbols.js#L32 | ||
| const REACT_LAZY_TYPE: symbol = Symbol.for("react.lazy"); | ||
| const REACT_MEMO_TYPE: symbol = Symbol.for("react.memo"); |
There was a problem hiding this comment.
Consider the risks of using React internals
Using React's internal symbols (react.lazy and react.memo) makes this implementation fragile. These symbols are not part of React's public API and could change in future versions without notice, potentially breaking this plugin.
Consider adding:
- Version checks or feature detection
- Fallback behavior if symbols are undefined
- Documentation about supported React versions
🤖 Prompt for AI Agents
In integrations/react/src/stable/lazyActivityPlugin.ts around lines 4 to 6, the
code uses React internal symbols react.lazy and react.memo which are not part of
the public API and may change unexpectedly. To fix this, add runtime checks to
verify these symbols exist before using them, implement fallback behavior if
they are undefined, and document the React versions supported by this plugin to
inform users of compatibility constraints.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
integrations/react/src/stable/stackflow.tsx (1)
146-151: Normalize plugin flattening (DRY) and avoid Infinity-as-0 hack scattered inlineCurrent inline flatten/cast is duplicated (also used in addPlugin) and relies on a TS hack. Centralize into a helper to improve readability and reduce mistakes.
Apply this diff here:
- const plugins: StackflowReactPlugin[] = [ - ...(options.plugins ?? []) - .flat(Number.POSITIVE_INFINITY as 0) - .map((p) => p as StackflowReactPlugin), - ]; + const plugins: StackflowReactPlugin[] = normalizePlugins(options.plugins);Add this helper (can be placed near the top of
stackflow()or file-level):function normalizePlugins<T extends BaseActivities>( entries: Array<StackflowPluginsEntry<T>> | undefined, ): StackflowReactPlugin[] { return (entries ?? []) .flat(Number.POSITIVE_INFINITY as 0) .map((p) => p as StackflowReactPlugin); }Optionally, reuse the same helper in
addPlugin()to remove duplication.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
integrations/react/src/stable/stackflow.tsx(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
- GitHub Check: Workers Builds: stackflow-docs
- GitHub Check: Create PR or release packages
- GitHub Check: Check whether the written test passes normally
- GitHub Check: Check the TypeScript typings
- GitHub Check: Build all packages
- GitHub Check: Cloudflare Pages
🔇 Additional comments (2)
integrations/react/src/stable/stackflow.tsx (2)
23-23: Import of internal plugin looks goodImport path and locality are appropriate for stable internals.
158-160: Verify plugin ordering relative to user pluginsAppending places the lazy plugin last. If other plugins hook onBeforePush/Replace, they may run before the lazy pause logic. Prepending ensures lazy-load pausing happens earliest.
If you keep append semantics, please confirm behavior with commonly used plugins (e.g., history sync) to ensure no regressions. Otherwise, switch to unshift as suggested above.
| const majorReactVersion = Number.parseInt(version); | ||
|
|
||
| /** | ||
| * TODO: This plugin depends on internal APIs of React. | ||
| * A proper solution (e.g. Suspense integration) should be implemented in the next major version. | ||
| */ | ||
| if (majorReactVersion >= 18 && majorReactVersion <= 19) { | ||
| plugins.push(lazyActivityPlugin(activityComponentMap)); | ||
| } | ||
|
|
There was a problem hiding this comment.
Tighten React version gate to 18.x and prepend plugin; add radix to parseInt
The plugin relies on React internals; enabling it for 19.x is risky. Also, to ensure it pauses navigation before other plugins, it should run first. Pass radix 10 to parseInt for robustness.
Apply this diff:
- const majorReactVersion = Number.parseInt(version);
+ const majorReactVersion = Number.parseInt(version, 10);
@@
- if (majorReactVersion >= 18 && majorReactVersion <= 19) {
- plugins.push(lazyActivityPlugin(activityComponentMap));
+ if (majorReactVersion >= 18 && majorReactVersion < 19) {
+ // Prepend so it can pause navigation early
+ plugins.unshift(lazyActivityPlugin(activityComponentMap));
}If you prefer extra safety, also log a dev-only warning when outside the supported range to aid debugging. I can provide a snippet.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const majorReactVersion = Number.parseInt(version); | |
| /** | |
| * TODO: This plugin depends on internal APIs of React. | |
| * A proper solution (e.g. Suspense integration) should be implemented in the next major version. | |
| */ | |
| if (majorReactVersion >= 18 && majorReactVersion <= 19) { | |
| plugins.push(lazyActivityPlugin(activityComponentMap)); | |
| } | |
| const majorReactVersion = Number.parseInt(version, 10); | |
| /** | |
| * TODO: This plugin depends on internal APIs of React. | |
| * A proper solution (e.g. Suspense integration) should be implemented in the next major version. | |
| */ | |
| if (majorReactVersion >= 18 && majorReactVersion < 19) { | |
| // Prepend so it can pause navigation early | |
| plugins.unshift(lazyActivityPlugin(activityComponentMap)); | |
| } |
🤖 Prompt for AI Agents
integrations/react/src/stable/stackflow.tsx around lines 152 to 161: tighten the
React version gate to only allow 18.x (e.g., check majorReactVersion === 18 or
majorReactVersion >= 18 && majorReactVersion < 19), pass radix 10 into
Number.parseInt(version, 10), and prepend the plugin to the plugins array
instead of pushing it (use unshift) so it runs before other plugins; optionally
add a dev-only warning (guarded by NODE_ENV !== 'production') when the React
version is outside the supported range to aid debugging.
|
/oc summarize |
|
This PR adds support for lazy-loaded React components in Stackflow by implementing an internal plugin that handles React.lazy and React.memo components. Key ChangesNew lazy activity plugin (
Plugin integration (
Changeset (
Technical ImplementationThe plugin works by:
This is backward-compatible and requires no configuration - it's automatically applied for supported React versions. |
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
integrations/react/src/stable/stackflow.tsx (1)
29-29: Tighten React gating to 18.x, prepend the plugin, and add radix to parseInt (optional dev warning)Given the plugin depends on React internals, be conservative: only enable on 18.x, run it before user plugins (so pause executes early), and parse React version with radix 10. Optional: warn in dev when disabled.
- import { version } from "react"; + import { version as reactVersion } from "react"; @@ - const majorReactVersion = Number.parseInt(version); + const majorReactVersion = Number.parseInt(reactVersion, 10); @@ - /** - * TODO: This plugin depends on internal APIs of React. - * A proper solution (e.g. Suspense integration) should be implemented in the next major version. - */ - if (majorReactVersion >= 18 && majorReactVersion <= 19) { - plugins.push(lazyActivityPlugin(activityComponentMap)); - } + /** + * TODO: This plugin depends on internal React internals. + * A proper solution (e.g., Suspense integration) should be implemented in the next major version. + */ + if (majorReactVersion >= 18 && majorReactVersion < 19) { + // Prepend so it can pause navigation before other plugins + plugins.unshift(lazyActivityPlugin(activityComponentMap)); + } else if (process.env.NODE_ENV !== "production") { + console.warn( + `@stackflow/react: lazyActivityPlugin disabled (React ${reactVersion} not in supported range 18.x).`, + ); + }Also applies to: 152-161
🧹 Nitpick comments (1)
integrations/react/src/stable/stackflow.tsx (1)
146-151: Simplify flattening and preserve generic typing for better type-safetyThe
flat(Number.POSITIVE_INFINITY as 0)trick works but is a bit opaque and drops the generic parameter. Consider simplifying and keeping the generic to help TS catch mismatches.- const plugins: StackflowReactPlugin[] = [ - ...(options.plugins ?? []) - .flat(Number.POSITIVE_INFINITY as 0) - .map((p) => p as StackflowReactPlugin), - ]; + const plugins: StackflowReactPlugin<NoInfer<T>>[] = []; + (options.plugins ?? []) + .flat(Infinity) + .forEach((p) => { + plugins.push(p as StackflowReactPlugin<NoInfer<T>>); + });Alternatively, extract a tiny helper for clarity:
function normalizePlugins<T extends BaseActivities>( entries: Array<StackflowPluginsEntry<T>>, ): StackflowReactPlugin<T>[] { return (entries.flat(Infinity) as unknown as StackflowReactPlugin<T>[]); }
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
integrations/react/src/stable/stackflow.tsx(2 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
integrations/react/src/stable/stackflow.tsx (2)
integrations/react/src/__internal__/StackflowReactPlugin.ts (1)
StackflowReactPlugin(4-41)integrations/react/src/stable/lazyActivityPlugin.ts (1)
lazyActivityPlugin(105-147)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Workers Builds: stackflow-docs
- GitHub Check: Cloudflare Pages
🔇 Additional comments (1)
integrations/react/src/stable/stackflow.tsx (1)
23-23: Import looks goodBrings in the internal plugin cleanly for use below. No issues.
|
/oc could you review this PR line-by-line via gh api command with gh cli? |
|
undefined is not an object (evaluating '(await client.session.chat({ |
No description provided.