feat(react/future): Add prefetch API for lazy activity component and loader data. #616
Conversation
🦋 Changeset detectedLatest commit: 55f23c8 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 prefetch hooks (usePrepare, useActivityPreparation), DataLoaderContext with per-activity cached loadData and TTL, ActivityComponentMapProvider for component lookup, updates loaderPlugin/createBeforeRouteHandler to use injected loadData and await loader/component promises, and refactors render wiring to use the new contexts. Changes
Sequence Diagram(s)sequenceDiagram
participant UI as Caller
participant Prep as usePrepare
participant ACM as ActivityComponentMapProvider
participant DL as loadData (stackflow)
participant ACT as Activity loader / component
UI->>Prep: prepare(activityName, params?)
Prep->>ACM: get component entry (check _load)
alt has _load
Prep->>ACT: call _load() (lazy component)
end
alt params & loader exist
Prep->>DL: loadData(activityName, params)
alt cache hit
DL-->>Prep: cached loaderData
else
DL->>ACT: invoke activity.loader(params)
ACT-->>DL: loaderData (cached + TTL)
DL-->>Prep: loaderData
end
end
Prep-->>UI: resolves
sequenceDiagram
participant User as User
participant Actions as Actions
participant Plugins as Plugins
participant LP as loaderPlugin
participant BR as beforeRouteHandler
participant DL as loadData
User->>Actions: push/replace(...)
Actions->>Plugins: dispatch
Plugins->>LP: onBeforePush/Replace
LP->>BR: createBeforeRouteHandler(input, loadData)
BR->>DL: loadData(activityName, params?) (if loader)
BR->>LP: await lazy component + loader promises
BR-->>LP: continue with actionParams (loaderData attached)
LP-->>Plugins: event updated
Plugins-->>Actions: proceed
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 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. ✨ 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 stackflow-demo with
|
| Latest commit: |
55f23c8
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://7a85fd26.stackflow-demo.pages.dev |
| Branch Preview URL: | https://route-preload.stackflow-demo.pages.dev |
commit: |
Deploying with
|
| Status | Name | Latest Commit | Preview URL | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! View logs |
stackflow-docs | 55f23c8 | Commit Preview URL | Aug 18 2025, 09:40 AM |
ENvironmentSet
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: 9
🔭 Outside diff range comments (1)
integrations/react/src/future/loader/loaderPlugin.tsx (1)
109-116: Reuse thenable detection for onBeforePush/onBeforeReplaceConsistently handle thenables here as well.
- const loaderDataPromise = - loaderData instanceof Promise ? loaderData : undefined; + const loaderDataPromise = + loaderData && typeof (loaderData as any).then === "function" + ? (loaderData as Promise<unknown>) + : undefined;
🧹 Nitpick comments (8)
.changeset/slick-heads-tan.md (1)
1-6: Clarify API names and options in the changesetSuggest explicitly listing:
- New action:
actions.prepare(activityName[, activityParams])- New hook:
useActivityPreparation(activityNames)- New option:
stackflow({ options: { loaderCacheMaxAge } })- One-liner on cache semantics and lazy component prefetch.
Improves discoverability for consumers scanning release notes.
integrations/react/src/future/useActivityPreparation.ts (1)
4-4: Consider supporting params to prefetch loader dataHook currently only preloads components. If loader prefetch requires params, consider an overload or accepting tuples like { name, params }.
Please confirm whether prepare(activityName) without params triggers loader prefetch; if not, the hook name/docs should clarify it only preloads components.
integrations/react/src/__internal__/PluginRenderer.tsx (1)
35-39: Optional: Guard for missing component mappingDefensive check helps fail fast with a clear message if a mapping is missing at runtime.
- const Activity = activityComponentMap[activity.name]; + const Activity = activityComponentMap[activity.name]; + if (!Activity) { + throw new Error( + `No component registered for activity "${activity.name}"`, + ); + }integrations/react/src/future/lazy.ts (1)
8-12: Nit: Name reflects content (promise vs value)cachedValue now holds a Promise; consider renaming to cachedPromise for clarity. No functional change.
integrations/react/src/future/stackflow.tsx (1)
102-105: Use referential equality for evictionDeep comparing whole entries (including data) is unnecessary and slower. The exact object reference can be removed safely.
- cache.filter((entry) => !isEqual(entry, newCacheEntry)), + cache.filter((entry) => entry !== newCacheEntry),integrations/react/src/future/makeActions.ts (2)
33-33: Reflect async loader in the type of loadDataActions.prepare wraps loader results in Promise.resolve. Make the type explicit for better DX.
- loadData: (activityName: string, activityParams: {}) => unknown, + loadData: (activityName: string, activityParams: {}) => unknown | Promise<unknown>,
110-114: Narrow to LazyActivityComponentType before calling _load for stronger type safetyThis avoids property access on a broad component type and prevents TS errors under stricter configs.
- if ("_load" in activityComponentMap[activityName]) { - const lazyComponent = activityComponentMap[activityName]; - - prefetchTasks.push(Promise.resolve(lazyComponent._load?.())); - } + if ("_load" in activityComponentMap[activityName]) { + const lazyComponent = + activityComponentMap[activityName] as LazyActivityComponentType<any>; + prefetchTasks.push(Promise.resolve(lazyComponent._load?.())); + }integrations/react/src/future/loader/loaderPlugin.tsx (1)
109-110: Minor: skip calling loadData when the activity has no loaderAvoids unnecessary cache entries/timers when no loader exists.
- const loaderData = loadData(activityName, activityParams); + const loaderData = matchActivity.loader + ? loadData(activityName, activityParams) + : undefined;
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (14)
.changeset/slick-heads-tan.md(1 hunks)integrations/react/src/__internal__/MainRenderer.tsx(1 hunks)integrations/react/src/__internal__/PluginRenderer.tsx(1 hunks)integrations/react/src/future/Actions.ts(1 hunks)integrations/react/src/future/ActivityComponentMapProvider.tsx(1 hunks)integrations/react/src/future/index.ts(1 hunks)integrations/react/src/future/lazy.ts(1 hunks)integrations/react/src/future/loader/DataLoaderContext.tsx(1 hunks)integrations/react/src/future/loader/index.ts(1 hunks)integrations/react/src/future/loader/loaderPlugin.tsx(3 hunks)integrations/react/src/future/makeActions.ts(3 hunks)integrations/react/src/future/stackflow.tsx(7 hunks)integrations/react/src/future/useActivityPreparation.ts(1 hunks)integrations/react/src/future/useFlow.ts(1 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 (5)
integrations/react/src/future/index.ts (1)
25-25: Public API: re-export useActivityPreparation — LGTMConsistent with existing hook exports. No concerns.
integrations/react/src/future/loader/index.ts (1)
3-3: Barrel export for DataLoaderContext — LGTMKeeps loader API cohesive. No issues spotted.
integrations/react/src/__internal__/PluginRenderer.tsx (2)
2-2: LGTM on switching to context-based activityComponentMapImporting useActivityComponentMap aligns with the new provider pattern.
17-17: PluginRenderer is always wrapped by ActivityComponentMapProviderVerified that:
- PluginRenderer (integrations/react/src/internal/PluginRenderer.tsx) is only rendered by MainRenderer.
- MainRenderer (integrations/react/src/internal/MainRenderer.tsx) is only used in future/stackflow.tsx.
- In integrations/react/src/future/stackflow.tsx (lines 221–228), ActivityComponentMapProvider fully encloses MainRenderer (and thus every PluginRenderer instance).
No further action required.
integrations/react/src/future/ActivityComponentMapProvider.tsx (1)
1-38: Solid provider/hook; error message is clearContext shape and guard are good. No issues.
There was a problem hiding this comment.
Actionable comments posted: 6
🔭 Outside diff range comments (3)
integrations/react/src/__internal__/MainRenderer.tsx (1)
2-2: Remove obsolete activityComponentMap prop and unused importThe component no longer consumes
activityComponentMap. Clean up the prop type and import to avoid confusion and unused types.-import type { ActivityComponentType } from "./ActivityComponentType"; @@ -interface MainRendererProps { - activityComponentMap: { - [key: string]: ActivityComponentType; - }; - initialContext: any; -} +interface MainRendererProps { + initialContext: any; +}Also applies to: 9-15
integrations/react/src/future/makeActions.ts (1)
50-63: replace() returns an ID that may not match the one used in the core actionYou compute activityId but call replace() with a different ID. This breaks consumer expectations and tracking.
- replace(activityName, activityParams, options) { - const activityId = makeActivityId(); + replace(activityName, activityParams, options) { + const activityId = options?.activityId ?? makeActivityId(); @@ - getCoreActions()?.replace({ - activityId: options?.activityId ?? makeActivityId(), + getCoreActions()?.replace({ + activityId, activityName, activityParams, skipEnterActiveState: parseActionOptions(options).skipActiveState, });integrations/react/src/future/loader/loaderPlugin.tsx (1)
90-113: Avoid unnecessary loadData calls and handle sync throws in route handlerOnly call loadData if a loader exists, and normalize sync throws to a rejected promise for consistent pause/resume behavior.
- input: StackflowInput<T, R>, - loadData: (activityName: string, activityParams: {}) => unknown, + input: StackflowInput<T, R>, + loadData: (activityName: string, activityParams: {}) => unknown, ): OnBeforeRoute { @@ - const loaderData = loadData(activityName, activityParams); - - const loaderDataPromise = - loaderData instanceof Promise ? loaderData : undefined; + let loaderData: unknown | undefined; + if (matchActivity.loader) { + try { + loaderData = loadData(activityName, activityParams); + } catch (err) { + loaderData = Promise.reject(err); + } + } + const loaderDataPromise = + loaderData instanceof Promise ? loaderData : undefined;
🧹 Nitpick comments (6)
integrations/react/src/future/useFlow.ts (1)
14-22: Stabilize Actions object with useMemo
makeActions(...)is executed on every render, changing theActionsobject identity and potentially causing unnecessary re-renders. Memoize based on inputs.+import { useMemo } from "react"; @@ - const actions = makeActions( - config, - () => coreActions, - activityComponentMap, - loadData, - ); - - return actions; + const actions = useMemo( + () => + makeActions( + config, + () => coreActions, + activityComponentMap, + loadData, + ), + [config, coreActions, activityComponentMap, loadData], + ); + + return actions;integrations/react/src/future/ActivityComponentMapProvider.tsx (1)
1-38: Solid context API; minor type-hardening optionalImplementation looks correct and aligns with the new access pattern. Optionally mark the map as readonly to prevent accidental mutation.
-const ActivityComponentMapContext = createContext< - | { - [activityName in RegisteredActivityName]: ActivityComponentType; - } - | null ->(null); +const ActivityComponentMapContext = createContext< + | Readonly<Record<RegisteredActivityName, ActivityComponentType>> + | null +>(null); @@ -type ActivityComponentMapProviderProps = PropsWithChildren<{ - value: { - [activityName in RegisteredActivityName]: ActivityComponentType; - }; -}>; +type ActivityComponentMapProviderProps = PropsWithChildren<{ + value: Readonly<Record<RegisteredActivityName, ActivityComponentType>>; +}>;integrations/react/src/__internal__/MainRenderer.tsx (1)
15-15: Optional: Narrow anyIf feasible, narrow
initialContext: anyto a more specific type orunknownto reduce implicit-anyusage.-const MainRenderer: React.FC<MainRendererProps> = ({ initialContext }) => { +const MainRenderer: React.FC<MainRendererProps> = ({ initialContext }) => {(Consider updating
MainRendererProps.initialContexttounknownor a concrete type.)integrations/react/src/future/loader/DataLoaderContext.tsx (1)
3-5: Unify and reuse the loader function type via a named aliasDefine a DataLoaderFn type and reuse it here to reduce duplication and keep signatures consistent across modules.
Apply:
export const DataLoaderContext = createContext< - ((activityName: string, activityParams: {}) => unknown) | null + DataLoaderFn | null >(null); @@ }: { - loadData: (activityName: string, activityParams: {}) => unknown; + loadData: DataLoaderFn; children: ReactNode; })Add near the top (outside the selected range):
export type DataLoaderFn = (activityName: string, activityParams: {}) => unknown;Also applies to: 11-13
integrations/react/src/future/makeActions.ts (1)
110-114: Type-narrow lazy components before calling _loadUse a type guard (or cast) so TS understands _load exists on lazy components.
- if ("_load" in activityComponentMap[activityName]) { - const lazyComponent = activityComponentMap[activityName]; - - prefetchTasks.push(Promise.resolve(lazyComponent._load?.())); - } + if ("_load" in activityComponentMap[activityName]) { + const lazyComponent = + activityComponentMap[activityName] as LazyActivityComponentType<any>; + prefetchTasks.push(Promise.resolve(lazyComponent._load?.())); + }If preferred, add a type guard outside the selected range and use it:
function isLazyActivityComponent( c: ActivityComponentType<any>, ): c is LazyActivityComponentType<any> { return typeof (c as any)._load === "function"; }integrations/react/src/future/stackflow.tsx (1)
70-73: Consider refreshing TTL on cache hits (sliding expiration)Returning a cached value does not extend its lifetime. If desired UX is “keep warm while accessed,” refresh expiry when a hit occurs.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (14)
.changeset/slick-heads-tan.md(1 hunks)integrations/react/src/__internal__/MainRenderer.tsx(1 hunks)integrations/react/src/__internal__/PluginRenderer.tsx(1 hunks)integrations/react/src/future/Actions.ts(1 hunks)integrations/react/src/future/ActivityComponentMapProvider.tsx(1 hunks)integrations/react/src/future/index.ts(1 hunks)integrations/react/src/future/lazy.ts(1 hunks)integrations/react/src/future/loader/DataLoaderContext.tsx(1 hunks)integrations/react/src/future/loader/index.ts(1 hunks)integrations/react/src/future/loader/loaderPlugin.tsx(3 hunks)integrations/react/src/future/makeActions.ts(3 hunks)integrations/react/src/future/stackflow.tsx(7 hunks)integrations/react/src/future/useActivityPreparation.ts(1 hunks)integrations/react/src/future/useFlow.ts(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (6)
integrations/react/src/future/useActivityPreparation.ts (2)
config/src/RegisteredActivityName.ts (1)
RegisteredActivityName(3-6)integrations/react/src/future/useFlow.ts (1)
useFlow(12-25)
integrations/react/src/future/Actions.ts (2)
config/src/RegisteredActivityName.ts (1)
RegisteredActivityName(3-6)config/src/InferActivityParams.ts (1)
InferActivityParams(5-8)
integrations/react/src/future/lazy.ts (1)
integrations/react/src/__internal__/StaticActivityComponentType.ts (1)
StaticActivityComponentType(3-5)
integrations/react/src/future/ActivityComponentMapProvider.tsx (1)
config/src/RegisteredActivityName.ts (1)
RegisteredActivityName(3-6)
integrations/react/src/future/makeActions.ts (5)
config/src/Config.ts (1)
Config(5-10)config/src/ActivityDefinition.ts (1)
ActivityDefinition(4-9)config/src/RegisteredActivityName.ts (1)
RegisteredActivityName(3-6)core/src/makeCoreStore.ts (1)
CoreStore(30-36)config/src/InferActivityParams.ts (1)
InferActivityParams(5-8)
integrations/react/src/future/stackflow.tsx (6)
config/src/ActivityDefinition.ts (1)
ActivityDefinition(4-9)config/src/RegisteredActivityName.ts (1)
RegisteredActivityName(3-6)integrations/react/src/future/loader/loaderPlugin.tsx (1)
loaderPlugin(9-79)integrations/react/src/future/ActivityComponentMapProvider.tsx (1)
ActivityComponentMapProvider(29-38)integrations/react/src/future/loader/DataLoaderContext.tsx (1)
DataLoaderProvider(7-19)integrations/react/src/future/makeActions.ts (1)
makeActions(27-119)
⏰ 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 (7)
integrations/react/src/future/index.ts (1)
25-25: LGTM! Export addition is correctly placed.The new
useActivityPreparationhook is properly exported in the hooks section, following the existing pattern and making it available to consumers of the public API.integrations/react/src/future/loader/index.ts (1)
3-3: LGTM! DataLoaderContext export is properly added.The export correctly exposes the
DataLoaderContextwhich is essential for the prefetch functionality's centralized data loading approach..changeset/slick-heads-tan.md (1)
1-6: LGTM! Changeset properly documents the feature addition.The changeset correctly specifies a minor version bump for
@stackflow/reactand accurately describes the prefetch API feature being added.integrations/react/src/future/Actions.ts (1)
29-32: LGTM! Well-designed prepare method signature.The new
preparemethod follows the established patterns:
- Uses the same generic constraint as other methods for type safety
- Optional
activityParamsprovides flexibility for different prefetch scenariosPromise<void>return type is appropriate for async prefetch operationsintegrations/react/src/__internal__/PluginRenderer.tsx (1)
2-2: Good move: eliminate prop drilling for activityComponentMapSwitching to
useActivityComponentMap()simplifies wiring and aligns with the new provider-based architecture.Also applies to: 17-17
integrations/react/src/__internal__/MainRenderer.tsx (1)
15-15: ActivityComponentMapProvider correctly wraps MainRendererConfirmed that in
integrations/react/src/future/stackflow.tsxthe<ActivityComponentMapProvider>(lines 221–228) wraps<MainRenderer>, ensuringuseActivityComponentMaphas the required context. No further changes needed here.integrations/react/src/future/stackflow.tsx (1)
223-226: Avoid passing activityComponentMap prop if now provided via contextYou’re providing the map via ActivityComponentMapProvider and also passing activityComponentMap to MainRenderer. Drop the prop to prevent API duplication/drift.
- <MainRenderer - initialContext={initialContext} - activityComponentMap={input.components} - /> + <MainRenderer initialContext={initialContext} />
orionmiz
force-pushed
the
route-preload
branch
from
b7b7bcb to
c61676b
Compare
orionmiz
force-pushed
the
route-preload
branch
from
9eba438 to
c61676b
Compare
|
/oc Run |
|
✅ Rebase completed successfully! I successfully:
The branch is now successfully rebased onto the latest main branch and all conflicts have been resolved. |
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
integrations/react/src/future/stackflow.tsx (1)
63-109: Don't cache when there's no loader; evict on rejected loader promises; clean up empty bucketsCurrently, activities without a loader are cached with data: undefined, and rejected Promises remain cached until TTL. This causes unnecessary cache entries and prevents retries after failures within the TTL window.
const loadData = (activityName: string, activityParams: {}) => { @@ - const loaderData = activityConfig.loader?.({ - params: activityParams, - config: input.config, - }); + if (!activityConfig.loader) { + // No loader for this activity: don't cache undefined + return undefined; + } + + const loaderData = activityConfig.loader({ + params: activityParams, + config: input.config, + }); const newCacheEntry = { params: activityParams, data: loaderData, }; if (cache) { cache.push(newCacheEntry); } else { loaderDataCacheMap.set(activityName, [newCacheEntry]); } + // If the loader rejects, evict this entry immediately to allow retrial + if (loaderData instanceof Promise) { + loaderData.catch(() => { + const cache = loaderDataCacheMap.get(activityName); + if (!cache) return; + const next = cache.filter((entry) => entry !== newCacheEntry); + if (next.length > 0) { + loaderDataCacheMap.set(activityName, next); + } else { + loaderDataCacheMap.delete(activityName); + } + }); + } + setTimeout(() => { const cache = loaderDataCacheMap.get(activityName); if (!cache) return; - loaderDataCacheMap.set( - activityName, - cache.filter((entry) => entry !== newCacheEntry), - ); + const next = cache.filter((entry) => entry !== newCacheEntry); + if (next.length > 0) { + loaderDataCacheMap.set(activityName, next); + } else { + loaderDataCacheMap.delete(activityName); + } }, input.options?.loaderCacheMaxAge ?? DEFAULT_LOADER_CACHE_MAX_AGE);
🧹 Nitpick comments (4)
integrations/react/src/future/makeActions.ts (2)
5-17: Simplify parseActionOptions; reduce branches while preserving semanticsCurrent logic returns the same result using multiple early returns. This can be simplified and made more readable.
Apply this diff:
-function parseActionOptions(options?: { animate?: boolean }) { - if (!options) { - return { skipActiveState: false }; - } - - const isNullableAnimateOption = options.animate == null; - - if (isNullableAnimateOption) { - return { skipActiveState: false }; - } - - return { skipActiveState: !options.animate }; -} +function parseActionOptions(options?: { animate?: boolean }) { + const animate = options?.animate; + // Skip transitions only when animate is explicitly false; otherwise don't skip. + return { skipActiveState: animate === false }; +}
49-77: Avoid recomputing parseActionOptions inside the loopMinor readability/perf nit: compute the first-skip value once outside the loop.
Apply this diff:
if (options) { _options = { ...options, }; } - for (let i = 0; i < _count; i += 1) { - getCoreActions()?.pop({ - skipExitActiveState: - i === 0 ? parseActionOptions(_options).skipActiveState : true, - }); - } + const skipFirstExit = parseActionOptions(_options).skipActiveState; + for (let i = 0; i < _count; i += 1) { + getCoreActions()?.pop({ + skipExitActiveState: i === 0 ? skipFirstExit : true, + }); + }integrations/react/src/future/usePrepare.ts (1)
11-16: Stabilize the returned function identity with useCallbackSmall ergonomics improvement: memoize the returned prepare to avoid referential changes across re-renders.
-export function usePrepare(): Prepare { - const config = useConfig(); - const loadData = useDataLoader(); - const activityComponentMap = useActivityComponentMap(); - - return async function prepare<K extends RegisteredActivityName>( +export function usePrepare(): Prepare { + const config = useConfig(); + const loadData = useDataLoader(); + const activityComponentMap = useActivityComponentMap(); + + const prepare = useCallback( + async function prepare<K extends RegisteredActivityName>(Outside this hunk, add the import:
import { useCallback } from "react";And at the end of the function return the memoized callback:
}, [config, loadData, activityComponentMap]); return prepare;integrations/react/src/future/stackflow.tsx (1)
44-46: Configurable cache TTL is a solid choiceoptions.loaderCacheMaxAge adds needed flexibility. Consider documenting units (ms) in JSDoc for clarity.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
.changeset/slick-heads-tan.md(1 hunks)integrations/react/src/future/index.ts(1 hunks)integrations/react/src/future/makeActions.ts(1 hunks)integrations/react/src/future/stackflow.tsx(6 hunks)integrations/react/src/future/useActivityPreparation.ts(1 hunks)integrations/react/src/future/useFlow.ts(1 hunks)integrations/react/src/future/usePrepare.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- integrations/react/src/future/useActivityPreparation.ts
- integrations/react/src/future/useFlow.ts
- .changeset/slick-heads-tan.md
🧰 Additional context used
🧬 Code Graph Analysis (3)
integrations/react/src/future/usePrepare.ts (4)
config/src/RegisteredActivityName.ts (1)
RegisteredActivityName(3-6)config/src/InferActivityParams.ts (1)
InferActivityParams(5-8)integrations/react/src/future/loader/DataLoaderContext.tsx (1)
useDataLoader(21-29)integrations/react/src/__internal__/ActivityComponentMapProvider.tsx (1)
useActivityComponentMap(13-21)
integrations/react/src/future/makeActions.ts (3)
core/src/makeCoreStore.ts (1)
CoreStore(29-35)integrations/react/src/future/Actions.ts (1)
Actions(6-29)integrations/react/src/__internal__/activity/makeActivityId.ts (1)
makeActivityId(3-5)
integrations/react/src/future/stackflow.tsx (7)
integrations/react/src/stable/stackflow.tsx (1)
stackflow(130-401)config/src/ActivityDefinition.ts (1)
ActivityDefinition(4-9)config/src/RegisteredActivityName.ts (1)
RegisteredActivityName(3-6)integrations/react/src/__internal__/ActivityComponentType.ts (1)
ActivityComponentType(4-6)integrations/react/src/future/loader/loaderPlugin.tsx (1)
loaderPlugin(10-80)integrations/react/src/__internal__/ActivityComponentMapProvider.tsx (1)
ActivityComponentMapProvider(29-38)integrations/react/src/future/loader/DataLoaderContext.tsx (1)
DataLoaderProvider(7-19)
⏰ 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)
- GitHub Check: Build all packages
- GitHub Check: Create PR or release packages
- GitHub Check: Check whether the written test passes normally
- GitHub Check: Check the TypeScript typings
- GitHub Check: Workers Builds: stackflow-docs
🔇 Additional comments (8)
integrations/react/src/future/makeActions.ts (3)
21-34: push implementation LGTMGenerates a single activityId, forwards animate semantics correctly, and returns the id. Clean and consistent.
35-48: replace implementation LGTMUsing a local activityId (preferring provided one) improves readability and avoids duplicated expressions. Animate handling matches push.
19-21: makeActions aligns with the current Actions type
- Checked
integrations/react/src/future/Actions.ts: the exportedActionstype only declarespush,replace, andpop.- Confirmed
integrations/react/src/future/makeActions.tsimplements exactly those three methods.- There is no
prepareon theActionstype, so no missing implementation or type/runtime gap today.If you later extend
Actionsto include apreparemethod, remember to updatemakeActionsto return it as well.integrations/react/src/future/index.ts (1)
12-13: Public API: new exports look correct; ensure they’re included in the published bundleExposing useActivityPreparation and usePrepare from the future barrel aligns with the new prefetch API. Confirm rollup/tsup configs include these files so they ship in the package entrypoints.
integrations/react/src/future/usePrepare.ts (1)
16-27: Good validation of unknown activitiesThrowing for unknown activities prevents silent no-ops and surfaces config mistakes early.
integrations/react/src/future/stackflow.tsx (3)
14-16: Context introduction LGTMImporting react-fast-compare and ActivityComponentMapProvider aligns with deep param cache lookups and context-driven component access.
118-118: Plugin wiring looks correctPassing loadData into loaderPlugin coordinates centralized data resolution with the plugin hooks.
221-225: Provider composition is appropriateWrapping MainRenderer with ActivityComponentMapProvider and DataLoaderProvider ensures hooks like useActivityComponentMap and useDataLoader are available throughout the tree, including usePrepare.
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
integrations/react/src/__internal__/utils/isThenable.ts(1 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). (5)
- GitHub Check: Workers Builds: stackflow-docs
- GitHub Check: Create PR or release packages
- GitHub Check: Check the TypeScript typings
- GitHub Check: Check whether the written test passes normally
- GitHub Check: Build all packages
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
integrations/react/src/__internal__/utils/isPromiseLike.ts (2)
2-7: Broaden thenable detection to include callable thenables (Promises/A+).Promises/A+ defines a thenable as an object or function with a then method. Current check excludes callable thenables. This is a defensive improvement and avoids rare false negatives.
Apply this diff:
export function isPromiseLike(value: unknown): value is PromiseLike<unknown> { return ( - typeof value === "object" && + (typeof value === "object" || typeof value === "function") && value !== null && "then" in value && - typeof value.then === "function" + typeof (value as { then: unknown }).then === "function" ); }
1-8: Add a brief JSDoc for clarity and future maintainability.This utility will be widely used in loader handling; documenting the intended semantics (thenable detection) helps avoid misuses and regressions.
+/** + * Returns true if the given value is a thenable (Promise-like): an object or function + * with a `then` method. Useful when a loader may return either a value or a promise. + */ export function isPromiseLike(value: unknown): value is PromiseLike<unknown> { return ( (typeof value === "object" || typeof value === "function") && value !== null && "then" in value && typeof (value as { then: unknown }).then === "function" ); }
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
integrations/react/src/__internal__/utils/isPromiseLike.ts(1 hunks)integrations/react/src/future/loader/loaderPlugin.tsx(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- integrations/react/src/future/loader/loaderPlugin.tsx
⏰ 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: Check whether the written test passes normally
- GitHub Check: Check the TypeScript typings
- GitHub Check: Build all packages
- GitHub Check: Create PR or release packages
- GitHub Check: Cloudflare Pages
🔇 Additional comments (1)
integrations/react/src/__internal__/utils/isPromiseLike.ts (1)
1-8: All thenable checks consolidated – no remaining legacy or ad-hoc “.then” checks foundI ran a repository-wide search and confirmed:
- No references to a legacy
isThenablehelper.- No ad-hoc checks for
"then" inortypeof …\.then === "function"outside of the new utility itself.The codebase consistently uses
isPromiseLikefor thenable detection.
No further changes are needed here.
ENvironmentSet
changed the title
ENvironmentSet
requested review from
orionmiz and
tonyfromundefined
and removed request for
orionmiz and
tonyfromundefined
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (2)
integrations/react/src/future/stackflow.tsx (2)
79-86: Skip caching when an activity has no loaderCaching an entry with undefined data for activities without a loader causes unnecessary memory usage and timers. Return early instead. This mirrors earlier feedback.
- const loaderData = activityConfig.loader?.({ - params: activityParams, - config: input.config, - }); + if (!activityConfig.loader) { + // No loader for this activity: don't cache undefined + return undefined; + } + + const loaderData = activityConfig.loader({ + params: activityParams, + config: input.config, + });Also applies to: 88-93
99-103: Delete the map entry when the cache bucket becomes emptyAvoid retaining empty arrays in the map to reduce memory footprint and keep the structure clean. This also aligns with prior feedback to evict by reference and cleanly remove buckets.
- loaderDataCacheMap.set( - activityName, - cache.filter((entry) => entry !== newCacheEntry), - ); + const next = cache.filter((entry) => entry !== newCacheEntry); + if (next.length > 0) { + loaderDataCacheMap.set(activityName, next); + } else { + loaderDataCacheMap.delete(activityName); + }
🧹 Nitpick comments (2)
config/src/ActivityLoader.ts (1)
4-7: Tighten the type surface: mark TTL as readonly; consider generics over the return type
- Marking loaderCacheMaxAge as readonly communicates intent and prevents accidental mutation elsewhere.
- Consider making the return type generic (e.g., <TData = unknown>) instead of any to improve type-safety end-to-end.
-export type ActivityLoader<ActivityName extends RegisteredActivityName> = { - (args: ActivityLoaderArgs<ActivityName>): any; - loaderCacheMaxAge?: number; -}; +export type ActivityLoader<ActivityName extends RegisteredActivityName, TData = unknown> = { + (args: ActivityLoaderArgs<ActivityName>): TData; + readonly loaderCacheMaxAge?: number; +};Note: If you adopt the generic return type here, propagate it through call sites where ActivityLoader is referenced.
integrations/react/src/future/stackflow.tsx (1)
60-62: Narrow the types for loader cache keys and paramsUsing RegisteredActivityName and ActivityBaseParams improves type safety across the stack and matches the config types.
- const loaderDataCacheMap = new Map<string, { params: {}; data: unknown }[]>(); - const loadData = (activityName: string, activityParams: {}) => { + const loaderDataCacheMap = new Map< + RegisteredActivityName, + { params: ActivityBaseParams; data: unknown }[] + >(); + const loadData = ( + activityName: RegisteredActivityName, + activityParams: ActivityBaseParams, + ) => {
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
config/src/ActivityLoader.ts(1 hunks)integrations/react/src/future/stackflow.tsx(5 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). (5)
- GitHub Check: Workers Builds: stackflow-docs
- GitHub Check: Build all packages
- GitHub Check: Check whether the written test passes normally
- GitHub Check: Create PR or release packages
- GitHub Check: Check the TypeScript typings
🔇 Additional comments (2)
integrations/react/src/future/stackflow.tsx (2)
128-129: Plugin placement looks correctAppending loaderPlugin after user-provided plugins ensures it runs after historySyncPlugin when present.
231-235: Provider composition LGTMSupplying ActivityComponentMapProvider and DataLoaderProvider around MainRenderer matches the new architecture and reduces prop plumbing.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
integrations/react/src/future/useActivityPreparation.ts (1)
5-11: Optionally accept activityParams and avoid floating PromisesSince
preparesupports(activityName[, activityParams]), you can pass through optional params now and explicitly discard the returned Promise to appease “no-floating-promises” linters. Also consider a readonly array to discourage mutation.Apply:
export function useActivityPreparation( - activities: { activityName: RegisteredActivityName }[], -) { + activities: ReadonlyArray<{ + activityName: RegisteredActivityName; + activityParams?: unknown; + }>, +): void { const prepare = usePrepare(); - for (const { activityName } of activities) { - prepare(activityName); + for (const { activityName, activityParams } of activities) { + void prepare(activityName, activityParams); } }Notes:
- If you prefer stronger typing for
activityParams, we can introduce a mapped type later without breaking callers..changeset/slick-heads-tan.md (1)
5-7: Tighten wording and include cache TTL detailMinor grammar/style fixes and add the default cache TTL + configuration knob to make the release notes more actionable.
-Add prefetch API for lazy activity component and loader data. -- A hook `usePrepare()` which returns `prepare(activityName[, activityParams])` is added for navigation warmup. -- A hook `useActivityPreparation(activities)` for preparing navigations inside a component is added. +Add a prefetch API for lazy activity components and their loader data. +- New: `usePrepare()` returns `prepare(activityName[, activityParams])` for navigation warm-up. +- New: `useActivityPreparation(activities)` prepares navigations inside a component. Each item is `{ activityName }`. +- Preloaded loader data is cached for 30 seconds by default; configure the max age via `options.loaderCacheMaxAge`.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
.changeset/slick-heads-tan.md(1 hunks)integrations/react/src/future/useActivityPreparation.ts(1 hunks)
🧰 Additional context used
🪛 LanguageTool
.changeset/slick-heads-tan.md
[grammar] ~6-~6: There might be a mistake here.
Context: ...arams])is added for navigation warmup. - A hookuseActivityPreparation(activitie...
(QB_NEW_EN)
⏰ 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: Check whether the written test passes normally
- GitHub Check: Create PR or release packages
- GitHub Check: Build all packages
- GitHub Check: Check the TypeScript typings
- GitHub Check: Workers Builds: stackflow-docs
- GitHub Check: Cloudflare Pages
🔇 Additional comments (1)
integrations/react/src/future/useActivityPreparation.ts (1)
4-12: Scalable argument shape looks goodSwitching to an object array (
{ activityName }[]) makes the hook extensible without breaking changes later. Matches the earlier discussion.
usePrepare()which returnsprepare(activityName[, activityParams])is added for navigation warmup.prepare(activityName[, activityParams]): Direct stackflow to prepare to navigate to given activity. This makes stackflow preload code of given activity's component if it is lazy loaded one. This also makes stackflow to preload any data fetched by pre-defined loader in stackflow config for that activity if and only if activity params are given.loader(loaderFn[, config])while supplingloaderCacheMaxAgeoption.useActivityPreparation(activities)for preparing navigations inside a component is added.