Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
3 changes: 3 additions & 0 deletions .config/mise/conf.d/tasks-test.toml
Original file line number Diff line number Diff line change
Expand Up @@ -16,13 +16,16 @@ COVERAGE_THRESHOLD_STATEMENTS = "{{ vars.coverage_threshold_statements }}"
description = "Run tests in watch mode"
alias = "t"
run = "vp test"
env = { VITE_CONNECT_LOGGER = "false" }

[tasks."test:run"]
description = "Run tests once"
alias = "tr"
run = "vp test run"
env = { VITE_CONNECT_LOGGER = "false" }

[tasks."test:coverage"]
description = "Run tests with coverage"
alias = "tcov"
run = "vp test run --coverage"
env = { VITE_CONNECT_LOGGER = "false" }
88 changes: 88 additions & 0 deletions .storybook/abortErrorGuard.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,88 @@
import { addGlobalExtension, isAbort, isAction, withCallHook } from '@reatom/core'

interface CollectedAbortError {
actionName: string
message: string
}

export interface DrainAbortError extends CollectedAbortError {
count: number
}

interface ExpectedAbortError {
actionName?: RegExp | string
message?: RegExp | string
}

const collected: CollectedAbortError[] = []

export function clearAbortErrors() {
collected.length = 0
}

export function drainAbortErrors(): DrainAbortError[] {
const grouped = new Map<string, DrainAbortError>()

for (const error of collected) {
const key = `${error.actionName}\u0000${error.message}`
const existing = grouped.get(key)
if (existing) existing.count += 1
else grouped.set(key, { ...error, count: 1 })
}

collected.length = 0
return [...grouped.values()]
}

const isMatch = (actual: string, expected: RegExp | string | undefined) => {
if (!expected) return true
return typeof expected === 'string' ? actual === expected : expected.test(actual)
}

const escapeRegExp = (value: string) => value.replaceAll(/[.*+?^${}()|[\]\\]/g, '\\$&')

export const formatAbortErrors = (errors: DrainAbortError[]) =>
errors.map((e) => ` - ${e.actionName}: ${e.message} ×${e.count}`).join('\n')

export function assertOnlyExpectedAbortErrors(
expected: ExpectedAbortError,
reason = 'Expected Reatom AbortErrors',
) {
const errors = drainAbortErrors()
const unexpected = errors.filter(
(error) =>
!isMatch(error.actionName, expected.actionName) || !isMatch(error.message, expected.message),
)
if (unexpected.length > 0) {
throw new Error(`${reason} included unexpected AbortErrors:\n${formatAbortErrors(unexpected)}`)
}
}

export async function assertExpectedRouteLoaderTeardownAbort(routeName: string) {
await Promise.resolve()
await new Promise<void>((resolve) => queueMicrotask(resolve))
assertOnlyExpectedAbortErrors(
{
actionName: new RegExp(`${escapeRegExp(routeName)}.*\\.loader\\.onReject$`),
message: /unmatch/,
},
`Expected ${routeName} matched-route teardown AbortErrors`,
)
}

addGlobalExtension((target) => {
if (isAction(target) && target.name.endsWith('.onReject')) {
target.extend(
withCallHook((payload) => {

Check warning on line 76 in .storybook/abortErrorGuard.ts

View workflow job for this annotation

GitHub Actions / test

High CRAP score (moderate)

Function '<arrow>' has a CRAP score of 30.0 (threshold: 30.0). • Severity: moderate • Cyclomatic: 5 • Cognitive: 3 • CRAP: 30.0 (threshold: 30.0) • Lines: 9 CRAP combines complexity with coverage: high CRAP means changes here carry high risk. Consider adding tests, simplifying the function, or both.
const error = (payload as { error?: unknown })?.error
if (error && isAbort(error)) {
collected.push({
actionName: target.name,
message: String((error as Error).message ?? error),
})
}
}),
)
}
return target
})
29 changes: 24 additions & 5 deletions .storybook/preview.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,10 @@ import { reatomContext } from '@reatom/react'
import addonA11y from '@storybook/addon-a11y'
import { definePreview } from '@storybook/react-vite'
import { initialize, mswLoader } from 'msw-storybook-addon'

import { clearAbortErrors, drainAbortErrors, formatAbortErrors } from './abortErrorGuard'
// oxlint-disable-next-line no-restricted-imports
import { useMemo, type PropsWithChildren } from 'react'
import { useEffect, useMemo, type PropsWithChildren } from 'react'

import { handlers } from '#app/mocks/handlers'
import { setAuthenticatedForTest } from '#entities/auth'
Expand All @@ -31,10 +33,18 @@ function ReatomDecorator({
authenticated = true,
}: PropsWithChildren<{ authenticated?: boolean; initialPath?: string }>) {
const frame = useMemo(() => {
const nextFrame = setupStorybookUrl(initialPath)
nextFrame.run(() => setAuthenticatedForTest(authenticated ? authMockSession : null))
return nextFrame
return setupStorybookUrl(initialPath, () => {
setAuthenticatedForTest(authenticated ? authMockSession : null)
})
}, [authenticated, initialPath])

useEffect(() => {
return () => {
clearAbortErrors()
queueMicrotask(clearAbortErrors)
}
}, [frame])

return <reatomContext.Provider value={frame}>{children}</reatomContext.Provider>
}

Expand Down Expand Up @@ -62,12 +72,21 @@ const preview = definePreview({
},
// fallow-ignore-next-line complexity
beforeEach: async ({ globals }) => {
if (!import.meta.env['VITEST']) return
clearAbortErrors()
if (!(globalThis as Record<string, unknown>)['__vitest_worker__']) return
const { page } = await import('vite-plus/test/browser')
const viewportGlobal = globals['viewport'] as { value?: string } | string | undefined
const viewportName = typeof viewportGlobal === 'string' ? viewportGlobal : viewportGlobal?.value
const viewport = (viewportName ? getViewportSize(viewportName) : null) ?? FALLBACK_VIEWPORT
await page.viewport(viewport.width, viewport.height)
return () => {
const errors = drainAbortErrors()
if (errors.length > 0) {
throw new Error(
`Reatom AbortErrors detected during story test:\n${formatAbortErrors(errors)}`,
)
}
}
},
})

Expand Down
6 changes: 5 additions & 1 deletion .storybook/setupStorybookUrl.ts
Original file line number Diff line number Diff line change
@@ -1,13 +1,17 @@
import { context, noop, urlAtom, withChangeHook } from '@reatom/core'

const originalHref = window.location.href
export const setupStorybookUrl = (initialPath = '') => {
export const setupStorybookUrl = (initialPath = '', beforeNavigate?: () => void) => {
const frame = context.start()
frame.run(() => {
// Configure urlAtom for Storybook: routing state works internally but
// the iframe URL stays fixed so Storybook remains happy.
urlAtom.sync.set(() => noop)
urlAtom.extend(withChangeHook(() => void window.history.replaceState({}, '', originalHref)))
// Run pre-navigation setup (e.g. auth) BEFORE urlAtom.go so that
// route matching and loader evaluation happen only once with the
// correct state, avoiding concurrent loader abort errors.
beforeNavigate?.()
const base = import.meta.env.BASE_URL ?? ''
urlAtom.go(base + initialPath)
})
Expand Down
3 changes: 3 additions & 0 deletions bun.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 3 additions & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -67,5 +67,8 @@
"workerDirectory": [
"public"
]
},
"patchedDependencies": {
"@reatom/core@1001.0.0": "patches/@reatom%2Fcore@1001.0.0.patch"
}
}
30 changes: 30 additions & 0 deletions patches/@reatom%2Fcore@1001.0.0.patch
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
diff --git a/dist/index.js b/dist/index.js
index b3110cb9bbe3b771c3b3fa5e9afbd8fd95d4d04b..485372c98b801fd07a9ddfe43d45d7c560dc36fb 100644
--- a/dist/index.js
+++ b/dist/index.js
@@ -7126,14 +7126,14 @@ const onEvent = (target, type, cb, options) => {
//#region src/web/url.ts
/** Create the URL atom with the new Reatom API. */
let urlAtom = atom(null, "urlAtom").extend(withMiddleware(() => (next, ...params) => next(...params) ?? urlAtom.init()), withInitHook(() => {
- for (const [, routeAtom] of Object.entries(urlAtom.routes)) routeAtom.loader();
+ for (const [, routeAtom] of Object.entries(urlAtom.routes)) if (routeAtom()) routeAtom.loader();
}, "effect"), withParams((update, replace = false) => {
let url = top().state;
let newUrl = typeof update === "function" ? update(url ?? urlAtom.init()) : update;
if (newUrl.href === url?.href) return url;
if (url !== newUrl) {
_enqueue(() => {
- for (const [, routeAtom] of Object.entries(urlAtom.routes)) routeAtom.loader();
+ for (const [, routeAtom] of Object.entries(urlAtom.routes)) if (routeAtom()) routeAtom.loader();
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Preserve loader aborts when routes stop matching

When navigating away from a route with an in-flight loader, this guard skips calling that route's loader after routeAtom() becomes null. That removes the proactive recomputation that used to enter the loader, see the unmatch state, and trigger the withAbort/reject path; with the new isSomeLoaderPending short-circuit, unmatched loaders are also no longer kept subscribed via pending(). As a result, slow requests for the previous route can continue and fulfill into route.loader.data() after the route is no longer active instead of being aborted on navigation.

Useful? React with 👍 / 👎.

}, "compute");
if (STACK[STACK.length - 2]?.atom !== urlAtom.syncFromSource) urlAtom.sync()(newUrl, replace);
}
@@ -7572,7 +7572,7 @@ let reatomRoute = createRouteFactory(urlAtom);
* route
*/
const is404 = computed(() => Object.values(urlAtom.routes).every((route) => !route()), "is404");
-const isSomeLoaderPending = computed(() => Object.values(urlAtom.routes).some((route) => route.loader.pending() > 0), "isSomeLoaderPending");
+const isSomeLoaderPending = computed(() => Object.values(urlAtom.routes).some((route) => route.match() && route.loader.pending() > 0), "isSomeLoaderPending");
//#endregion
//#region src/routing/searchParams.ts
const isSubpath = (currentPath, targetPath) => !targetPath || targetPath[targetPath.length - 1] === "*" ? `${currentPath}/`.startsWith(targetPath.slice(0, -1)) : `${currentPath}/` === targetPath;
112 changes: 112 additions & 0 deletions src/app/integration/Articles.detail.stories.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,112 @@
import preview from '#.storybook/preview'
import { App } from '#app/App'
import { articleDetail } from '#entities/article/mocks/handlers'
import { articlesActor as I } from '#pages/articles/testing'
import { heading, role } from '#shared/test'

const meta = preview.meta({
title: 'Integration/Articles/Detail',
component: App,
parameters: {
layout: 'fullscreen',
initialPath: 'articles/1',
},
loaders: [(ctx) => I.init(ctx)],
})

export default meta

export const HandlesArticleDetailServerError = meta.story({
name: 'Article Detail Server Error',
play: () => I.waitExit(role('status')),
parameters: {
msw: {
handlers: { articleDetail: articleDetail.error },
},
},
})

HandlesArticleDetailServerError.test(
'shows error state when article detail request fails',
async () => {
await I.scope(role('main'), async () => {
await I.seeDetailError()
})
},
)

HandlesArticleDetailServerError.test('keeps detail error state when retry also fails', async () => {
await I.scope(role('main'), async () => {
await I.seeDetailError()
await I.retry()
await I.waitExit(role('status'))
await I.seeDetailError()
})
})

export const RecoversAfterArticleDetailRetry = meta.story({
name: 'Article Detail Retry Success',
play: () => I.waitExit(role('status')),
parameters: {
msw: {
handlers: { articleDetail: articleDetail.retrySucceeds() },
},
},
})

RecoversAfterArticleDetailRetry.test('loads article detail after retry succeeds', async () => {
await I.scope(role('main'), async () => {
await I.seeDetailError()
await I.retry()
await I.waitExit(role('status'))
await I.see(heading('Quarterly report').wait())
await I.seeArticleDetail('Quarterly report')
})
})

export const HandlesArticleDetailServerErrorMobile = meta.story({
name: 'Article Detail Server Error (Mobile)',
globals: { viewport: { value: 'sm', isRotated: false } },
parameters: HandlesArticleDetailServerError.input.parameters,
play: () => I.waitExit(role('status')),
})

HandlesArticleDetailServerErrorMobile.test(
'[mobile] shows error state when article detail request fails',
async () => {
await I.scope(role('main'), async () => {
await I.seeDetailError()
})
},
)

export const KeepsLoadingWhenArticleDetailNeverResolves = meta.story({
name: 'Article Detail Loading State',
parameters: {
msw: {
handlers: { articleDetail: articleDetail.loading },
},
},
})

KeepsLoadingWhenArticleDetailNeverResolves.test(
'shows detail loading state while article detail is pending',
async () => {
const detail = await I.see(role('main'))
await I.seeDetailLoading(detail)
},
)

export const KeepsLoadingWhenArticleDetailNeverResolvesMobile = meta.story({
name: 'Article Detail Loading State (Mobile)',
globals: { viewport: { value: 'sm', isRotated: false } },
parameters: KeepsLoadingWhenArticleDetailNeverResolves.input.parameters,
})

KeepsLoadingWhenArticleDetailNeverResolvesMobile.test(
'[mobile] shows detail loading state while article detail is pending',
async () => {
const detail = await I.see(role('main'))
await I.seeDetailLoading(detail)
},
)
49 changes: 49 additions & 0 deletions src/app/integration/Articles.direct-url.stories.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
import preview from '#.storybook/preview'
import { App } from '#app/App'
import { articlesActor as I } from '#pages/articles/testing'
import { role } from '#shared/test'

const meta = preview.meta({
title: 'Integration/Articles/Direct URL',
component: App,
parameters: {
layout: 'fullscreen',
initialPath: 'articles/1',
},
loaders: [(ctx) => I.init(ctx)],
})

export default meta

export const DirectUrlNavigation = meta.story({
name: 'Direct URL to Article',
play: () => I.waitExit(role('status')),
})

DirectUrlNavigation.test('loads article detail directly from URL', async () => {
await I.seeArticleDetail('Quarterly report')
await I.seeArticleDetailContent()
})

export const DirectUrlNotFound = meta.story({
name: 'Direct URL to Missing Article',
parameters: { initialPath: 'articles/missing-42' },
play: () => I.waitExit(role('status')),
})

DirectUrlNotFound.test('shows not-found state for missing article URL', async () => {
await I.scope(role('main'), async () => {
await I.seeArticleNotFound('missing-42')
})
})

export const DirectUrlNavigationMobile = meta.story({
name: 'Direct URL to Article (Mobile)',
globals: { viewport: { value: 'sm', isRotated: false } },
parameters: { initialPath: 'articles/1' },
play: () => I.waitExit(role('status')),
})

DirectUrlNavigationMobile.test('[mobile] loads article detail directly from URL', async () => {
await I.seeArticleDetail('Quarterly report')
})
Loading
Loading