Repository navigation
refactor(DropdownMenu): a failed sheet preload is the preload's result, not an empty catch - #9069
Conversation
…t, not an empty catch meta#8478 stopped a failed preload of the bottom sheet's action list from raising an unhandled rejection with `loadActionList().catch(() => {})`. An empty handler also hides any other error in that chain. preloadMenuBottomSheetActionList now resolves to whether the list loaded: {isLoaded: true}, or {isLoaded: false, error} with the loader's error. It never rejects, and nothing swallows an error. DropdownMenu's effect starts it with `void`; after a failure the first open loads the list, as before. lazyMenuBottomSheet.test.ts checks that result, along with no unhandled rejection and a retry on the next call. A [chore] changeset: callers see no change.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
meta-cla
Bot
added
the
CLA Signed
|
AI review status for this pull request.
|
PR Analysis Report📚 Storybook Preview🧪 Sandbox PreviewComponent Size
Sizes are gzip. Import counts the JavaScript the import loads eagerly; CSS counts the package stylesheet rules referenced by the modules it can load from its own package, lazily loaded ones included. Bundle Size SummaryGzip figures measure each package's entry file, not the code an import pulls in.
Accessibility AuditStatus: The accessibility audit did not run on this pull request. It runs after merges to Generated by PR Enrichment workflow | Storybook | Sandbox | View full report |
![astracat-bot[bot]](https://avatars.githubusercontent.com/in/4796231?s=60&v=4){kind=small}
There was a problem hiding this comment.
No changes needed: the sheet preload's failure now comes back as a typed result instead of an empty .catch, and nothing changes for people using a sheet menu. The updated test covers the failure result and the retry on the next call, and the DropdownMenu tests, typecheck and lint pass.
[Automated review]
cixzhang
added
the
ci:run
Follows #8478.
Maintainer impact
#8478 stopped a failed preload of a bottom-sheet DropdownMenu's action list from raising an unhandled rejection, using
loadActionList().catch(() => {}). An empty handler hides every error in that chain, not only the expected failed load, so a real defect there would go unnoticed.What led to it: a sweep of recent changes for error handlers that catch and default.
Intended invariant
A sheet preload's failure is its result, never a rejection and never swallowed:
preloadMenuBottomSheetActionListresolves{isLoaded: true}, or{isLoaded: false, error}with the loader's error. As before, a failed load is not kept, so the next open loads the list.Change
lazyMenuBottomSheet.ts: the preload returns that result (anActionListPreloadtype, internal to DropdownMenu).DropdownMenu.tsx: its effect starts the preload withvoid; after a failure, the first open loads the list.lazyMenuBottomSheet.test.ts: checks the result, no unhandled rejection, and a retry on the next call.[chore]changeset: callers see no change.Options considered:
lazy().Prior art: no open pull request touches
preloadMenuBottomSheetActionList,lazyMenuBottomSheet.tsor anActionListPreloadtype (searched by those names and "sheet preload").Evidence
lazyMenuBottomSheet.test.tsfails; it passes here.pnpm test:ui(16,454 passed).Scope
Testing
pnpm exec vitest run packages/core/src/DropdownMenu/lazyMenuBottomSheet.test.ts: fails on main, passes here.Local gates at 45046fe: build, typecheck, lint,
pnpm test:ui(16,454),pnpm test:node(all but #8520's two knowncache.test.mjsfailures, and two of main's load-sensitive cases,ci-scope-pr-diff.test.mjsandcomponent-size.test.mjs: 30 s timeouts at box load 70–85; they pass alone on main fd0aba8 and this head), CLI (47), Storybook, the DropdownMenu and Menu accessibility specs in Chromium (228), Prettier, changeset coverage, and the knowledge, changeset and repo checks.