fix(website): stop reading union-only fields off heterogeneous data - #15282
Merged
Conversation
mcpDemoPrompts was declared `as const satisfies readonly McpDemoPrompt[]`, so each element kept its exact literal type and the array became a union of nine shapes. Reading the optional via/variants/stacked off that union fails because no single member declares all three. Annotating the array as readonly McpDemoPrompt[] restores the intended optional fields, and matches how drops.ts declares its data. DropsSection read media.poster without narrowing DropMedia on its type discriminant, and poster only exists on the video member. Both were invisible because .vue files are not type-checked today.
🌐 Website E2ETip All tests passed.
🔗 Website PreviewWebsite Preview: https://comfy-website-preview-pr-15282.vercel.app This commit: https://website-frontend-f97tl2fnu-comfyui.vercel.app Last updated: 2026-08-14T22:23:34Z for |
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThe drops template now passes poster data only for video media. The MCP demo prompt list now uses an explicit readonly ChangesDrops media handling
MCP prompt typing
Estimated code review effort: 2 (Simple) | ~10 minutes Merge Risk: ⚪ Minimal · up to This PR makes localized type-safety fixes without changing intended runtime behavior, so no actionable merge-blocking risk remains after normal checks and review. Suggested reviewers: 🚥 Pre-merge checks | ✅ 6✅ Passed checks (6 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
github-actions
Bot
added
the
risk:R1
This was referenced Aug 14, 2026
christian-byrne
marked this pull request as ready for review
August 15, 2026 01:17
dosubot
Bot
added
the
size:XS
DrJKL
approved these changes
Aug 15, 2026
| toolKey: 'mcp.hero.demoToolSetExtension', | ||
| result: 'set_extension.png' | ||
| } | ||
| ] as const satisfies readonly McpDemoPrompt[] |
Contributor
There was a problem hiding this comment.
The inference here is slightly different.
christian-byrne
added a commit
that referenced
this pull request
Aug 16, 2026
…#15300) *PR Created by the Glary-Bot Agent* --- ## Summary `astro/tsconfigs/strictest` layers nine options on top of the `strict` preset the app already uses. Measured each against `apps/website`: seven report zero errors once six dead declarations are removed. This turns those seven on. ## Changes - **What**: enables `noUnusedLocals`, `noUnusedParameters`, `noImplicitReturns`, `noFallthroughCasesInSwitch`, `noImplicitOverride`, `allowUnreachableCode: false`, `allowUnusedLabels: false`, and deletes the six declarations that blocked them: - `data/drops.ts` — `FEATURED_BADGE` (unused `LocalizedText`) - `layouts/BaseLayout.astro` — unused `Locale` type import - `pages/careers.astro` + `pages/zh-CN/careers.astro` — `siteUrl` destructured but never read - `scripts/generate-models.ts` — unused `RawModel` interface - `components/product/enterprise/TeamSection.vue` — `routes` computed and its `getRoutes` import - **Breaking**: none. Every deletion is a declaration with no reader. ## Review Focus **Measured, not guessed.** Error counts per flag against this app: | flag | errors | enabled here | |---|---:|:---:| | `noUnusedParameters` | 0 | yes | | `noImplicitReturns` | 0 | yes | | `noFallthroughCasesInSwitch` | 0 | yes | | `noImplicitOverride` | 0 | yes | | `allowUnreachableCode: false` | 0 | yes | | `allowUnusedLabels: false` | 0 | yes | | `noUnusedLocals` | 6 | yes (fixed here) | | `exactOptionalPropertyTypes` | **75** | no | | `noUncheckedIndexedAccess` | **202** | no | The last two are left off deliberately — each needs its own remediation pass, not a flag flip. `noUncheckedIndexedAccess` in particular would touch every array index and `Record` lookup in the app. `TeamSection.vue` is only reachable by `vue-tsc`, which this app does not run yet (that arrives in #15284). It is fixed here anyway so the flag stays clean once that lands. ## Verification `astro check` 0 errors with the flags on · 422 unit tests passing (1 pre-existing `minimaxMusic3` failure, also on `main`) · production build 595 pages · oxfmt clean. Confirmed the new flags introduce no `vue-tsc` regressions: `vue-tsc` reports the same 9 pre-existing SFC errors as `main` (same four files, same counts — those are what #15282/#15283 fix), and **zero** TS6133/TS6196/TS7027/TS7030 diagnostics, i.e. none attributable to these flags. This PR is therefore independent of that stack and can merge in any order relative to it. --------- Co-authored-by: Glary-Bot <glary-bot@users.noreply.github.com> Co-authored-by: Alexander Brown <drjkl@comfy.org>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
PR Created by the Glary-Bot Agent
Summary
Fixes 6 of the 9 pre-existing
vue-tscerrors inapps/website, both caused by reading a field that does not exist on every member of a union. First of three stacked PRs that end withvue-tscrunning in CI.Changes
templates/mcp/mcpDemoPrompts.ts— the array was declaredas const satisfies readonly McpDemoPrompt[].satisfieschecks assignability but preserves theas consttype, so the array became a union of nine distinct literal shapes rather thanMcpDemoPrompt[]. Since no single member declares all ofvia/variants/stacked, every read of those optional fields failed (5 errors inComfyMcpDemo.vue). Annotating asreadonly McpDemoPrompt[]restores the intended optional fields and matches howdata/drops.tsdeclares its data. No consumer depended on the literal types.templates/drops/DropsSection.vue—drop.media.posterwas read without narrowingDropMediaon itstypediscriminant;posteronly exists on thevideomember. Now narrowed withdrop.media.type === 'video' ? drop.media.poster : undefined.Review Focus
Neither change alters runtime behaviour:
DropsSection: fortype: 'video'the value still comes fromdrop.media.poster; for images the property access already evaluated toundefinedat runtime, which is exactly what the ternary now yields. Confirmed no drop indata/drops.tspasses a poster — all threevideoFor(...)calls take only(fileName, alt)— so the rendered output is byte-identical.mcpDemoPrompts: droppingas constonly widens literal types to their declared ones. The values are unchanged.These were invisible until now because
.vuefiles are type-checked by nothing in this repo — that gap is fixed in the third PR of this stack.Verification
astro check0 errors ·vue-tscdrops from 9 errors to 3 (the remaining 3 are fixed in the next PR) · 422 unit tests passing (1 pre-existingminimaxMusic3.jpegfailure, also fails onmain) · lint/format/knip clean · production build 595 pages.Manually verified the MCP hero demo in a production build (screenshot below) — all three optional fields still render exactly as before:
variants: 4→ Character Concepts shows 4 separate thumbnailsstacked: true→ Keyframe Board shows the offset layers behind the thumbnailvia→ NOTION / FIGMA / BLENDER badges render, and cards withoutvia(Set Extension) correctly show noneAlso loaded
/launches(DropsSection): 3 videos, 0 with aposterattribute (matching the data), 31 images, none with a brokensrc, and no page errors.Screenshots