Skip to content

fix(plugin-history-sync): only call fallbackActivity when no route matches #707

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
orionmiz merged 3 commits into from
Apr 30, 2026
+71 −13
Merged

fix(plugin-history-sync): only call fallbackActivity when no route matches #707

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
6c4ebbb
fix(plugin-history-sync): only call fallbackActivity when no route ma…
orionmiz Apr 30, 2026
59e721c
test(plugin-history-sync): narrow jest.fn return type for fallbackAct…
orionmiz Apr 30, 2026
eb2ef69
fix(plugin-history-sync): cache fallbackActivity result outside find …
orionmiz Apr 30, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/fix-fallback-activity-called-on-match.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@stackflow/plugin-history-sync": patch
---

Fix `fallbackActivity` callback being invoked on every initialization regardless of route matching outcome. Restored the pre-1.8.0 contract: the callback is now called only when no route matches `currentPath`. Apps that perform side effects in this callback (e.g. Sentry logging for unknown deep links) no longer fire on successful matches.
48 changes: 48 additions & 0 deletions extensions/plugin-history-sync/src/historySyncPlugin.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -170,6 +170,54 @@ describe("historySyncPlugin", () => {
expect(activeActivity(actions.getStack())?.params.title).toEqual("hello");
});

test("historySyncPlugin - 초기에 매칭하는 라우트가 있으면 fallbackActivity 콜백을 호출하지 않습니다", async () => {
history = createMemoryHistory({
initialEntries: ["/articles/123"],
});

const fallbackActivity = jest.fn(() => "Home");

stackflow({
activityNames: ["Home", "Article"],
plugins: [
historySyncPlugin({
history,
routes: {
Home: "/home",
Article: "/articles/:articleId",
},
fallbackActivity,
}),
],
});

expect(fallbackActivity).not.toHaveBeenCalled();
});

test("historySyncPlugin - 초기에 매칭하는 라우트가 없으면 fallbackActivity 콜백을 호출합니다", async () => {
history = createMemoryHistory({
initialEntries: ["/non-existent-path"],
});

const fallbackActivity = jest.fn(() => "Home");

stackflow({
activityNames: ["Home", "Article"],
plugins: [
historySyncPlugin({
history,
routes: {
Home: "/home",
Article: "/articles/:articleId",
},
fallbackActivity,
}),
],
});

expect(fallbackActivity).toHaveBeenCalled();
});
Comment on lines +173 to +220
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot Apr 30, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical | ⚡ Quick win

Make the fallback mocks accept the callback argument.

fallbackActivity is typed as (args: { initialContext: any }) => K, but both new tests use jest.fn(() => "Home"). That no-arg mock is not assignable here, which matches the TS2322 failure in CI. Please change both mocks to accept the parameter, e.g. jest.fn((_args) => "Home") or jest.fn(({ initialContext }) => "Home").

🛠️ Suggested fix 
-    const fallbackActivity = jest.fn(() => "Home");
+    const fallbackActivity = jest.fn((_args) => "Home");
@@
-    const fallbackActivity = jest.fn(() => "Home");
+    const fallbackActivity = jest.fn((_args) => "Home");
📝 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.

Suggested change
test("historySyncPlugin - 초기에 매칭하는 라우트가 있으면 fallbackActivity 콜백을 호출하지 않습니다", async () => {
history = createMemoryHistory({
initialEntries: ["/articles/123"],
});
const fallbackActivity = jest.fn(() => "Home");
stackflow({
activityNames: ["Home", "Article"],
plugins: [
historySyncPlugin({
history,
routes: {
Home: "/home",
Article: "/articles/:articleId",
},
fallbackActivity,
}),
],
});
expect(fallbackActivity).not.toHaveBeenCalled();
});
test("historySyncPlugin - 초기에 매칭하는 라우트가 없으면 fallbackActivity 콜백을 호출합니다", async () => {
history = createMemoryHistory({
initialEntries: ["/non-existent-path"],
});
const fallbackActivity = jest.fn(() => "Home");
stackflow({
activityNames: ["Home", "Article"],
plugins: [
historySyncPlugin({
history,
routes: {
Home: "/home",
Article: "/articles/:articleId",
},
fallbackActivity,
}),
],
});
expect(fallbackActivity).toHaveBeenCalled();
});
test("historySyncPlugin - 초기에 매칭하는 라우트가 있으면 fallbackActivity 콜백을 호출하지 않습니다", async () => {
history = createMemoryHistory({
initialEntries: ["/articles/123"],
});
const fallbackActivity = jest.fn((_args) => "Home");
stackflow({
activityNames: ["Home", "Article"],
plugins: [
historySyncPlugin({
history,
routes: {
Home: "/home",
Article: "/articles/:articleId",
},
fallbackActivity,
}),
],
});
expect(fallbackActivity).not.toHaveBeenCalled();
});
test("historySyncPlugin - 초기에 매칭하는 라우트가 없으면 fallbackActivity 콜백을 호출합니다", async () => {
history = createMemoryHistory({
initialEntries: ["/non-existent-path"],
});
const fallbackActivity = jest.fn((_args) => "Home");
stackflow({
activityNames: ["Home", "Article"],
plugins: [
historySyncPlugin({
history,
routes: {
Home: "/home",
Article: "/articles/:articleId",
},
fallbackActivity,
}),
],
});
expect(fallbackActivity).toHaveBeenCalled();
});
🧰 Tools
🪛 GitHub Actions: Build

[error] 189-189: TypeScript declaration build failed (tsc). TS2322: Type 'Mock<string, [], any>' is not assignable to type '(args: { initialContext: any; }) => "Home" | "Article"'.

🪛 GitHub Actions: Integration

[error] 189-189: tsc (build:dts) failed with TypeScript error TS2322: Type 'Mock<string, [], any>' is not assignable to type '(args: { initialContext: any; }) => "Home" | "Article"'.

🪛 GitHub Actions: Release

[error] 189-189: TS2322: Type 'Mock<string, [], any>' is not assignable to type '(args: { initialContext: any; }) => "Home" | "Article"'. (tsc --emitDeclarationOnly, step @stackflow/plugin-history-sync::tsc)

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@extensions/plugin-history-sync/src/historySyncPlugin.spec.ts` around lines
173 - 219, The tests use a fallbackActivity mock that must match the typed
signature (args: { initialContext: any }) => K; update both jest.fn mocks in the
failing tests to accept the callback arg (e.g. jest.fn((_args) => "Home") or
jest.fn(({ initialContext }) => "Home")) so the mock is assignable to the
fallbackActivity parameter passed into historySyncPlugin when calling stackflow;
ensure both occurrences in the tests that define fallbackActivity are changed
accordingly.


test("historySyncPlugin - actions.push() 후에, URL 상태가 알맞게 바뀝니다", async () => {
await actions.push({
activityId: "a1",
Expand Down
23 changes: 11 additions & 12 deletions extensions/plugin-history-sync/src/historySyncPlugin.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -228,22 +228,21 @@ export function historySyncPlugin<
}

const currentPath = resolveCurrentPath();
const fallbackActivityName = options.fallbackActivity({
initialContext,
const matchedActivityRoute = activityRoutes.find((activityRoute) => {
const template = makeTemplate(
activityRoute,
options.urlPatternOptions,
);
const activityParams = template.parse(currentPath);

return activityParams !== null;
});
const targetActivityRoute =
activityRoutes.find((activityRoute) => {
const template = makeTemplate(
activityRoute,
options.urlPatternOptions,
);
const activityParams = template.parse(currentPath);

return activityParams !== null;
}) ??
matchedActivityRoute ??
activityRoutes.find(
(activityRoute) =>
activityRoute.activityName === fallbackActivityName,
activityRoute.activityName ===
options.fallbackActivity({ initialContext }),
)!;
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Outdated
const pattern = new UrlPattern(
`${targetActivityRoute.path}(/)`,
Expand Down
Loading