fix: baseline typecheck stabilization for ui exports#100
Conversation
There was a problem hiding this comment.
2 issues found across 21 files
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="packages/ui/src/integrations/apps-sdk/index.ts">
<violation number="1" location="packages/ui/src/integrations/apps-sdk/index.ts:1">
P2: Duplicate barrel: `integrations/index.ts` contains the exact same 10 exports as this new canonical `apps-sdk/index.ts`. Consider having `integrations/index.ts` re-export from `./apps-sdk` (e.g. `export * from "./apps-sdk"`) to keep a single source of truth and avoid drift when exports are added or removed.</violation>
</file>
<file name="packages/ui/src/integrations/figma/ImageWithFallback/ImageWithFallback.tsx">
<violation number="1" location="packages/ui/src/integrations/figma/ImageWithFallback/ImageWithFallback.tsx:27">
P2: The required `alt` prop is discarded in the error fallback branch, replacing the meaningful accessible label with a generic "Error loading image" string. This undermines the accessibility intent stated in the interface's JSDoc. Consider incorporating the original `alt` so assistive technology users still know what the image was meant to convey.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| @@ -0,0 +1,10 @@ | |||
| export { AppsSDKUIProvider } from "@openai/apps-sdk-ui/components/AppsSDKUIProvider"; | |||
There was a problem hiding this comment.
P2: Duplicate barrel: integrations/index.ts contains the exact same 10 exports as this new canonical apps-sdk/index.ts. Consider having integrations/index.ts re-export from ./apps-sdk (e.g. export * from "./apps-sdk") to keep a single source of truth and avoid drift when exports are added or removed.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/ui/src/integrations/apps-sdk/index.ts, line 1:
<comment>Duplicate barrel: `integrations/index.ts` contains the exact same 10 exports as this new canonical `apps-sdk/index.ts`. Consider having `integrations/index.ts` re-export from `./apps-sdk` (e.g. `export * from "./apps-sdk"`) to keep a single source of truth and avoid drift when exports are added or removed.</comment>
<file context>
@@ -0,0 +1,10 @@
+export { AppsSDKUIProvider } from "@openai/apps-sdk-ui/components/AppsSDKUIProvider";
+export { Badge } from "@openai/apps-sdk-ui/components/Badge";
+export { Button } from "@openai/apps-sdk-ui/components/Button";
</file context>
| style={style} | ||
| > | ||
| <div className="flex items-center justify-center w-full h-full"> | ||
| <img src={ERROR_IMG_SRC} alt="Error loading image" {...rest} data-original-url={src} /> |
There was a problem hiding this comment.
P2: The required alt prop is discarded in the error fallback branch, replacing the meaningful accessible label with a generic "Error loading image" string. This undermines the accessibility intent stated in the interface's JSDoc. Consider incorporating the original alt so assistive technology users still know what the image was meant to convey.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/ui/src/integrations/figma/ImageWithFallback/ImageWithFallback.tsx, line 27:
<comment>The required `alt` prop is discarded in the error fallback branch, replacing the meaningful accessible label with a generic "Error loading image" string. This undermines the accessibility intent stated in the interface's JSDoc. Consider incorporating the original `alt` so assistive technology users still know what the image was meant to convey.</comment>
<file context>
@@ -0,0 +1,33 @@
+ style={style}
+ >
+ <div className="flex items-center justify-center w-full h-full">
+ <img src={ERROR_IMG_SRC} alt="Error loading image" {...rest} data-original-url={src} />
+ </div>
+ </div>
</file context>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d647c88790
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| } | ||
|
|
||
| export function ImageWithFallback(props: ImageWithFallbackProps) { | ||
| const [didError, setDidError] = useState(false); |
There was a problem hiding this comment.
Reset fallback state when src prop changes
Once didError is set to true, this component always renders the fallback branch and never attempts to load a new image URL, because the error state is not tied to src. In views where src can change (e.g., list virtualization, carousels, or async retries), a single transient failure permanently locks the instance to the error placeholder even after a valid URL is provided.
Useful? React with 👍 / 👎.
|
The latest updates on your projects. Learn more about Argos notifications ↗︎
|
Summary
packages/uipublic export barrels to canonicalintegrations/apps-sdk+integrations/figmamodulesapps-sdk-wrappersources and duplicate icon barrel exports that caused missing symbol + duplicate identifier failures@design-studio/icons->@design-studio/astudio-icons) and refresh lockfile@design-studio/runtime/@design-studio/tokenssource path aliases fromtsconfig.base.jsonand restorepackages/ui/tsconfig.jsonexclusions to stop cross-project TS6307 failuresValidation
pnpm install --frozen-lockfile --prod=falsepnpm -C packages/ui type-checkpnpm -C packages/runtime type-checkpnpm -C packages/tokens type-checkpnpm ds:matrix:checkpnpm lintpnpm format:checkpnpm test:policySummary by cubic
Stabilizes TypeScript across the UI package by making exports canonical and removing duplicate/broken icon barrels. Replaces the Apps SDK wrapper with direct vendor re-exports and fixes path alias issues so installs, type checks, and policy checks stay green.
Bug Fixes
Refactors
Written for commit d647c88. Summary will update on new commits.