chore: remove uncontroversial ts-expect-error suppressions#540
Merged
willeastcott merged 2 commits intoMay 27, 2026
Conversation
Three pure typing fixes that let us drop 12 of the existing `@ts-expect-error` markers without any runtime change: - Widen `attachElement` in the React wrapper base to `(HTMLElement | SVGElement | null, any?)` so it matches React's callback ref contract (allows null on unmount, and SVG for the Spinner wrapper). Matched in the LabelGroup override. - Change `ColorPicker.set values` parameter from `Array<number>` to `Array<any>` to match the `IBindable` interface contract — the body already handles array-of-arrays. - Annotate the `comboItems` literal in `GradientPicker` as `Record<number, string>` so the dynamic `comboItems[3] = 'Legacy'` assignment type-checks. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR removes several TypeScript suppression comments by aligning React wrapper ref typing and a couple of component typings with existing usage and interfaces.
Changes:
- Widens React wrapper
attachElementsignatures to support callback refs,null, and SVG elements. - Removes now-unneeded
@ts-expect-errorcomments from wrapper render refs. - Adjusts
ColorPicker.valuestyping andGradientPickercombo item typing.
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
src/components/Element/component.tsx |
Widens base attachElement callback ref type and removes base render suppression. |
src/components/LabelGroup/component.tsx |
Mirrors the widened attachElement override signature. |
src/components/Button/component.tsx |
Removes ref suppression. |
src/components/Canvas/component.tsx |
Removes ref suppression. |
src/components/ColorPicker/component.tsx |
Removes ref suppression. |
src/components/ColorPicker/index.ts |
Aligns values setter with IBindable array contract. |
src/components/Container/component.tsx |
Removes ref suppression. |
src/components/GradientPicker/component.tsx |
Removes ref suppression. |
src/components/GradientPicker/index.ts |
Adds a numeric record type for curve type combo items. |
src/components/InfoBox/component.tsx |
Removes ref suppression. |
src/components/Label/component.tsx |
Removes ref suppression. |
src/components/Spinner/component.tsx |
Removes SVG ref suppression. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
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
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.
Summary
Three pure typing fixes that let us drop 12 of the existing
// @ts-expect-errormarkers in pcui (38 → 26), with no runtime change and no public-API expansion.attachElementin the React wrapper base (src/components/Element/component.tsx) to(HTMLElement | SVGElement | null, any?)so it matches React's callback ref contract. The body already short-circuits onnull, and the wider DOM type covers Spinner's<svg>. Matched in the LabelGroup override. Drops the suppression above every<… ref={this.attachElement} />in Button, Canvas, ColorPicker, Container, Element, GradientPicker, InfoBox, Label, Spinner (9 sites).ColorPicker.set valuesto theIBindablecontract (src/components/ColorPicker/index.ts) — the parameter was declaredArray<number>but the body already handles array-of-arrays (theArray.isArray(value)branch). Changed toArray<any>to match the interface (2 sites).comboItemsasRecord<number, string>in src/components/GradientPicker/index.ts so the dynamiccomboItems[3] = 'Legacy'assignment type-checks (1 site).The remaining 26 suppressions all require a real API decision (visibility widening, dynamic-property data smell, or interface mismatches) and are intentionally left out of this PR.
Test plan
npm run build:types— tsc clean on bothtsconfig.jsonandreact/tsconfig.json(the removed markers were each suppressing an actual error before; tsc would fail if any deleted one weren't needed)npm run lint:js— cleannpm run build:es6— Rollup build greennpm test— 50/50 pass🤖 Generated with Claude Code