chore(ts): fix build:tsc pipeline in pnpm monorepo#1208
Merged
Conversation
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 50 minutes and 14 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
WalkthroughTightened and separated TypeScript typings (notably WPNotice vs Notice), updated many component and icon declarations with explicit JSX/React types, adjusted notice-related selectors/actions to validate statuses, and modified TypeScript config and package dependencies. All changes are type/signature-only except a few runtime casts; no functional behavior was added. Changes
Sequence Diagram(s)Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (3)
packages/icons/src/lib/mark-white.tsx (1)
6-6: Type annotation correctly fixes TS2742 build error.The explicit
JSX.Elementannotation resolves the inferred-type naming issue caused by pnpm hoisting of@types/react. This change is correct and aligns with the PR's objective to fix the failing build:tsc step.
Consider converting to function components for better React patterns.
While the current constant-based pattern works, converting these icons to function components would enable:
- Accepting dynamic props (e.g., custom className, aria labels, size)
- Better alignment with modern React patterns
- Improved reusability across different contexts
♻️ Example refactor to function component
-const MarkWhite: JSX.Element = ( +const MarkWhite = () => ( <SVG viewBox="0 0 179 179"This is not required for the current build fix but could be considered in a future refactor.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/icons/src/lib/mark-white.tsx` at line 6, Keep the explicit type annotation on the MarkWhite constant to preserve the TS2742 build fix (leave const MarkWhite: JSX.Element = (...) as-is); if you want the suggested refactor, replace the const with a function component named MarkWhite that accepts standard SVG/React props (e.g., props: React.SVGProps<SVGSVGElement> & { className?: string, ariaLabel?: string }), return the same SVG JSX from that function, and export it – ensure the function signature and return type align with React.FC or explicit JSX.Element to avoid reintroducing the type inference issue.packages/core-data/src/call-to-actions/actions.ts (1)
55-65: Type annotationn: Noticeis now semantically wrong after theNoticevsWPNoticesplit.
getNotices()returns WP-shaped notice records (matchingWPNotice), not theNoticeOptions-shapedNoticetype introduced in this PR. It still compiles because the fields you touch (id) overlap structurally, but the intent is confusing — and then.id?.startsWith(...)optional chain only makes sense underNotice(whereidis optional); onWPNotice.id(requiredstring) the?.is dead code.♻️ Suggested tightening
-import type { EditorId, Notice, WPNotice } from '../types'; +import type { EditorId, Notice, WPNotice } from '../types'; @@ - const fieldErrors = notices.filter( - ( n: Notice ) => n.id?.startsWith( `field-error-${ ctaId }-` ) - ); - fieldErrors.forEach( ( n: Notice ) => + const fieldErrors = notices.filter( ( n: WPNotice ) => + n.id.startsWith( `field-error-${ ctaId }-` ) + ); + fieldErrors.forEach( ( n: WPNotice ) => registry .dispatch( noticesStore ) .removeNotice( n.id, NOTICE_CONTEXT ) );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core-data/src/call-to-actions/actions.ts` around lines 55 - 65, The filter is typed as (n: Notice) but getNotices() returns WP-shaped notices (WPNotice); change the annotation in the fieldErrors filter to use WPNotice (or let inference pick it up) and remove the unnecessary optional chaining on id (use n.id.startsWith(...)); update the predicate in the fieldErrors declaration that references getNotices(), registry, noticesStore, NOTICE_CONTEXT and ensure the dispatch.removeNotice call still uses n.id (typed as string) so TypeScript reflects the WPNotice shape.packages/core-data/src/types/notices.ts (1)
101-117: LocalNoticeActiontype is incomplete compared to upstream@wordpress/notices.The type omits several properties supported by
@wordpress/noticesv5.29.0:
className?: stringnoDefaultClasses?: booleandisabled?: booleanvariant?: 'primary' | 'secondary' | 'link'If any consumer attempts to pass these valid upstream properties, TypeScript will reject them. Consider aligning the type with upstream or re-exporting the WordPress type directly.
The
url: string | null→url?: stringnarrowing mentioned in PR objectives has no current callsites explicitly passingurl: null, so runtime impact is minimal.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core-data/src/types/notices.ts` around lines 101 - 117, The local NoticeAction type is missing fields from upstream `@wordpress/notices`; update the NoticeAction type declaration to include optional properties className?: string, noDefaultClasses?: boolean, disabled?: boolean, and variant?: 'primary' | 'secondary' | 'link' (or re-export the upstream NoticeAction type from `@wordpress/notices` to keep parity) so consumers passing those upstream-supported props won't be rejected; adjust the export in packages/core-data/src/types/notices.ts accordingly, keeping url as optional if you prefer the narrowed type.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@packages/core-data/src/call-to-actions/actions.ts`:
- Around line 55-65: The filter is typed as (n: Notice) but getNotices() returns
WP-shaped notices (WPNotice); change the annotation in the fieldErrors filter to
use WPNotice (or let inference pick it up) and remove the unnecessary optional
chaining on id (use n.id.startsWith(...)); update the predicate in the
fieldErrors declaration that references getNotices(), registry, noticesStore,
NOTICE_CONTEXT and ensure the dispatch.removeNotice call still uses n.id (typed
as string) so TypeScript reflects the WPNotice shape.
In `@packages/core-data/src/types/notices.ts`:
- Around line 101-117: The local NoticeAction type is missing fields from
upstream `@wordpress/notices`; update the NoticeAction type declaration to include
optional properties className?: string, noDefaultClasses?: boolean, disabled?:
boolean, and variant?: 'primary' | 'secondary' | 'link' (or re-export the
upstream NoticeAction type from `@wordpress/notices` to keep parity) so consumers
passing those upstream-supported props won't be rejected; adjust the export in
packages/core-data/src/types/notices.ts accordingly, keeping url as optional if
you prefer the narrowed type.
In `@packages/icons/src/lib/mark-white.tsx`:
- Line 6: Keep the explicit type annotation on the MarkWhite constant to
preserve the TS2742 build fix (leave const MarkWhite: JSX.Element = (...)
as-is); if you want the suggested refactor, replace the const with a function
component named MarkWhite that accepts standard SVG/React props (e.g., props:
React.SVGProps<SVGSVGElement> & { className?: string, ariaLabel?: string }),
return the same SVG JSX from that function, and export it – ensure the function
signature and return type align with React.FC or explicit JSX.Element to avoid
reintroducing the type inference issue.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 34c85ec9-d51f-4c49-9f17-94dc54fc64aa
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (22)
packages/core-data/src/call-to-actions/actions.tspackages/core-data/src/popups/actions.tspackages/core-data/src/types/notices.tspackages/icons/src/lib/block-manager.tsxpackages/icons/src/lib/block.tsxpackages/icons/src/lib/check-all.tsxpackages/icons/src/lib/custom-redirect.tsxpackages/icons/src/lib/filter-lines.tsxpackages/icons/src/lib/gears.tsxpackages/icons/src/lib/incognito.tsxpackages/icons/src/lib/license-key.tsxpackages/icons/src/lib/locked-user.tsxpackages/icons/src/lib/mark-colored.tsxpackages/icons/src/lib/mark-light.tsxpackages/icons/src/lib/mark-retro.tsxpackages/icons/src/lib/mark-white.tsxpackages/icons/src/lib/mark.tsxpackages/icons/src/lib/monitor.tsxpackages/icons/src/lib/permissions.tsxpackages/icons/src/lib/protected-message.tsxpackages/icons/src/lib/upgrade.tsxtsconfig.base.json
Three related fixes so build:tsc stops failing on pre-existing config
drift:
- Empty out the ambient 'types: [node, react, jest, @testing-library/
jest-dom]' array. In a pnpm-hoisted monorepo this forces every
package to resolve type libs it doesn't have as direct deps, which
fails. Packages that need globals (jquery, etc) already declare
them in their own tsconfig. TS auto-includes every @types/*
reachable via node_modules when 'types' is absent or empty, which
is the correct monorepo behavior.
- Widen the test-file exclude pattern to match actual conventions
(__tests__/ and *.test.{ts,tsx}) instead of the unused 'test/'.
Test files shouldn't participate in type-build; they're compiled
by jest.
- Silence the TS 5.x deprecation warning for 'moduleResolution: node'
with 'ignoreDeprecations: 5.0'. Migrating to 'bundler' breaks
type-root resolution under pnpm hoisting; defer that to its own
dedicated task before TS 7.0 removes the escape hatch entirely.
tsc TS2742: inferred types couldn't be named portably because pnpm hoists @types/react into a versioned .pnpm path that's not reachable from the emitted .d.ts. Explicit annotations let tsc emit the type name without a reference to the hoisted path.
Several call sites spread a local Notice object into WP's
createNotice(status, content, options) as the options arg — but the
local Notice type had drifted from upstream NoticeOptions in three
ways that tsc flagged once the test-file mask was lifted:
1. Notice.status was widened with '| string', erasing the narrow
union WP expects ('error' | 'warning' | 'success' | 'info').
Drop the widening.
2. Notice extended WPNotice, so spreading it into createNotice
options passed extras (status, content, spokenMessage, __unstableHTML)
that aren't part of the options contract. Redefine Notice to
match NoticeOptions exactly — distinct shape, not a superset of
WPNotice.
3. NoticeAction.url/onClick typed as 'string | null' / 'Function |
null' didn't match WP's 'string | undefined' / '() => void'.
Realign.
Action handlers that used Notice['status']/Notice['content'] for
their input param types now reference WPNotice directly — Notice no
longer carries those fields since they're function arguments rather
than options.
Note: this is a type-only refactor of the emitted public types for
consumers of @popup-maker/core-data. NoticeAction.url going from
'string | null' to 'string | undefined' is a technical breaking
change for TS consumers, but runtime semantics are unchanged (null
was already effectively optional).
Previously typescript was only pulled in transitively via storybook and stylelint peer deps. Makes tsc reliably resolvable from root node_modules/.bin, which helps workspace-level invocations and gives pnpm a stable hoist target for the TS compiler itself.
getNotices() returns WPNotice[] (the shape with required id: string), not Notice (which is the NoticeOptions shape). Correcting the type annotation removes the unnecessary optional chain on n.id.
Previous commit added typescript@5.7.3 as root devDep but the lock regeneration missed resolving the webpack-cli variant of webpack, causing frozen-lockfile install to fail in CI. Full lockfile refresh produces a consistent tree.
danieliser
force-pushed
the
fix/tsconfig-build-tsc-compliance
branch
from
bb3c673 to
0fb3e9f
Compare
Earlier WPNotice rename broke column alignment of the @param block for createNotice() in both call-to-actions/actions.ts and popups/ actions.ts. jsdoc/check-line-alignment caught it in CI.
Once Phase 1 fixed the TS5107 deprecation errors, tsc got far enough to emit .d.ts files and hit TS2742 'cannot be named without a reference to .pnpm/@types+react@18.3.28/...' across 17 exported components. Explicit return-type annotations let tsc emit portable type names instead of trying to follow the pnpm-hoisted path. Special cases: - controlled-tab-panel/index.tsx: extract withInstanceId(TabPanel) to a named const with React.ComponentType annotation. - url-control/suggestion.tsx: annotate outer const with React.ForwardRefExoticComponent so the forwardRef return type is nameable. - confirm-dialogue/index.tsx: annotation is JSX.Element | null since the component returns null when no callback is provided.
…742) Same pnpm-hoisted @types/react pattern as the components package. use-fields.tsx: FieldsContext annotated with React.Context<FieldsContextType> so the createContext return is nameable.
AppContent, AppHeader, AppLayout: standard JSX.Element return-type. slots/index.ts: the four createSlotFill() destructures couldn't emit portable types for their Fill/Slot properties. Broke each destructure into an intermediate const and annotated the re-exports with shared SlotFillFill / SlotFillSlot type aliases derived from ReturnType<typeof createSlotFill>.
…-editor, cta-admin (TS2742)
Continuing Phase 2 after components/fields/layout: annotate every
remaining TS2742 hit in the monorepo with explicit JSX.Element
(or JSX.Element | null where the component can return null).
Also:
- block-editor: add missing @wordpress/{i18n,edit-post,plugins,
api-fetch} deps for popup-title-panel.tsx, fix two pre-existing
type issues in that file (double-cast to unknown, drop invalid
icon: null, add null return type).
- cta-editor: add missing @wordpress/notices + @types/node deps,
widen withDataStore/withModal/withQueryParams HOC signatures to
their emitted ComponentType form, annotate Editor, fix Context
destructure.
- cta-admin: annotate 33 exported components. context.tsx splits
the generic Context destructure into explicit Provider/Consumer
consts. index.tsx casts registry to RegistryProvider's expected
value type (pre-existing mismatch). list-view/notices.tsx now
compiles thanks to core-data selector fix below.
- core-data: correct getNotices() / getNoticeById() selectors to
return WPNotice (full shape with id/status/content) instead of
Notice (NoticeOptions shape). The prior cast to Notice was a
hold-over from the Phase 1 type realignment and broke every
downstream consumer that read notice.id / .status / .content.
Prettier reformatting of 12 files whose manual annotations didn't match the repo's prettier config (line wrapping, indentation).
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
packages/core-data/src/popups/selectors.ts (1)
339-353: Type cast narrowsstatusunsafely.
noticesStore.getNotices()returns notices whosestatusis typed asstringupstream. Casting to the narrowerWPNotice[](with a 4-literalstatusunion) is a one-way assertion: if any third-party plugin creates a notice inNOTICE_CONTEXTwith a non-standard status, downstream consumers will assume an invalid literal. Either keepstatusoptional/stringonWPNotice, or filter by known statuses before casting.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core-data/src/popups/selectors.ts` around lines 339 - 353, The code unsafely casts noticesStore.getNotices( NOTICE_CONTEXT ) to WPNotice[] in getNotices and getNoticeById which narrows status to a 4-literal union; instead either (A) widen the WPNotice.status type to accept string (update the WPNotice definition) so the cast is safe, or (B) validate and filter the returned notices before casting: call noticesStore.getNotices( NOTICE_CONTEXT ), filter entries by an isValidStatus(status) predicate that only keeps known statuses, then cast/map to WPNotice for return; apply the same pattern in both getNotices and getNoticeById.packages/core-data/src/types/notices.ts (1)
4-47: Minor:WPNotice.statusis looser than@wordpress/notices.
@wordpress/noticestypesstatusas required on a notice record returned from the store, but here it isstatus?:. SincegetNotices()always returns a notice with a status set, consider making it required to catch accidentalundefinedreads at call sites (and tighten the cast inselectors.ts).Proposed tightening
- status?: 'warning' | 'success' | 'error' | 'info'; + status: 'warning' | 'success' | 'error' | 'info';🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core-data/src/types/notices.ts` around lines 4 - 47, The WPNotice.status field is currently optional but should be required to match `@wordpress/notices` and the guarantees from getNotices(); change the WPNotice interface so status is non-optional (status: 'warning' | 'success' | 'error' | 'info') and then update any code that creates WPNotice instances to always supply a status; also tighten the casts/usages in selectors.ts (remove the permissive undefined casts and assume status exists) so callers no longer rely on an optional status and any places that previously omitted status are updated to provide a default/explicit value.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/cta-editor/src/editor/index.ts`:
- Line 1: The import in editor/index.ts currently pulls ComponentType from
'@wordpress/element' but should import ComponentType from 'react' to match the
other HOC files; update the import statement that references ComponentType so it
imports from 'react' (change the import source for ComponentType) to maintain
consistency with with-query-params.tsx, with-modal.tsx, and with-data-store.tsx.
In `@packages/fields/src/lib/html.tsx`:
- Line 4: The HtmlField component currently renders raw HTML via
dangerouslySetInnerHTML using the content prop; add an explicit note or enforce
sanitization: either add a clear comment above the HtmlField export stating that
content must be pre-sanitized server-side (e.g., via wp_kses in the REST
controller) and must never contain unsanitized user input, or (preferred) call a
client-side sanitizer utility on content before passing it to
dangerouslySetInnerHTML (reference the HtmlField component and its content prop)
so the component always outputs safe HTML regardless of upstream handling.
---
Nitpick comments:
In `@packages/core-data/src/popups/selectors.ts`:
- Around line 339-353: The code unsafely casts noticesStore.getNotices(
NOTICE_CONTEXT ) to WPNotice[] in getNotices and getNoticeById which narrows
status to a 4-literal union; instead either (A) widen the WPNotice.status type
to accept string (update the WPNotice definition) so the cast is safe, or (B)
validate and filter the returned notices before casting: call
noticesStore.getNotices( NOTICE_CONTEXT ), filter entries by an
isValidStatus(status) predicate that only keeps known statuses, then cast/map to
WPNotice for return; apply the same pattern in both getNotices and
getNoticeById.
In `@packages/core-data/src/types/notices.ts`:
- Around line 4-47: The WPNotice.status field is currently optional but should
be required to match `@wordpress/notices` and the guarantees from getNotices();
change the WPNotice interface so status is non-optional (status: 'warning' |
'success' | 'error' | 'info') and then update any code that creates WPNotice
instances to always supply a status; also tighten the casts/usages in
selectors.ts (remove the permissive undefined casts and assume status exists) so
callers no longer rely on an optional status and any places that previously
omitted status are updated to provide a default/explicit value.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 6caf5aa2-b9b1-4d6d-83d7-9cced691a226
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (108)
package.jsonpackages/block-editor/package.jsonpackages/block-editor/src/formats/popup-trigger/index.tsxpackages/block-editor/src/formats/popup-trigger/inline.tsxpackages/block-editor/src/formats/popup-trigger/trigger-popover/popup-trigger-editor.tsxpackages/block-editor/src/formats/popup-trigger/trigger-popover/popup-trigger-viewer.tsxpackages/block-editor/src/plugins/popup-title-panel.tsxpackages/components/src/lib/confirm-dialogue/index.tsxpackages/components/src/lib/controlled-tab-panel/index.tsxpackages/components/src/lib/controlled-tab-panel/tab-button.tsxpackages/components/src/lib/cta-select-control/index.tsxpackages/components/src/lib/device-toggle/index.tsxpackages/components/src/lib/entity-select-control/index.tsxpackages/components/src/lib/field-panel/index.tsxpackages/components/src/lib/field-row/index.tsxpackages/components/src/lib/freeform-control/index.tsxpackages/components/src/lib/full-screen-panel/index.tsxpackages/components/src/lib/list-table/index.tsxpackages/components/src/lib/popup-select-control/index.tsxpackages/components/src/lib/radio-buttons-control/index.tsxpackages/components/src/lib/searchable-multicheck-control/index.tsxpackages/components/src/lib/text-highlight/index.tsxpackages/components/src/lib/url-control/index.tsxpackages/components/src/lib/url-control/suggestion.tsxpackages/core-data/src/call-to-actions/actions.tspackages/core-data/src/call-to-actions/selectors.tspackages/core-data/src/popups/actions.tspackages/core-data/src/popups/selectors.tspackages/core-data/src/types/notices.tspackages/cta-admin/src/App.tsxpackages/cta-admin/src/components/app-header/index.tsxpackages/cta-admin/src/components/list-view/header.tsxpackages/cta-admin/src/components/list-view/index.tsxpackages/cta-admin/src/components/list-view/notices.tsxpackages/cta-admin/src/components/list/bulk-actions/index.tsxpackages/cta-admin/src/components/list/filters/index.tsxpackages/cta-admin/src/components/list/index.tsxpackages/cta-admin/src/components/list/options/index.tsxpackages/cta-admin/src/components/list/quick-actions/index.tsxpackages/cta-admin/src/context.tsxpackages/cta-admin/src/index.tsxpackages/cta-admin/src/registries/list-bulk-actions/delete.tsxpackages/cta-admin/src/registries/list-bulk-actions/disable.tsxpackages/cta-admin/src/registries/list-bulk-actions/enable.tsxpackages/cta-admin/src/registries/list-bulk-actions/export.tsxpackages/cta-admin/src/registries/list-bulk-actions/trash.tsxpackages/cta-admin/src/registries/list-filters/status.tsxpackages/cta-admin/src/registries/list-filters/type.tsxpackages/cta-admin/src/registries/list-options/export.tsxpackages/cta-admin/src/registries/list-options/import.tsxpackages/cta-admin/src/registries/list-quick-actions/delete.tsxpackages/cta-admin/src/registries/list-quick-actions/edit.tsxpackages/cta-admin/src/registries/list-quick-actions/export.tsxpackages/cta-admin/src/registries/list-quick-actions/trash.tsxpackages/cta-editor/package.jsonpackages/cta-editor/src/components/debug-notices.tsxpackages/cta-editor/src/editor/components/header-actions.tsxpackages/cta-editor/src/editor/components/header-options.tsxpackages/cta-editor/src/editor/hocs/with-data-store.tsxpackages/cta-editor/src/editor/hocs/with-modal.tsxpackages/cta-editor/src/editor/hocs/with-query-params.tsxpackages/cta-editor/src/editor/index.tspackages/cta-editor/src/registries/header-actions/history.tsxpackages/cta-editor/src/registries/header-actions/status.tsxpackages/cta-editor/src/registries/header-options/delete.tsxpackages/cta-editor/src/registries/tabs/general.tsxpackages/cta-editor/tsconfig.jsonpackages/fields/src/hooks/use-fields.tsxpackages/fields/src/lib/checkbox.tsxpackages/fields/src/lib/color.tsxpackages/fields/src/lib/custom-select.tsxpackages/fields/src/lib/date.tsxpackages/fields/src/lib/field.tsxpackages/fields/src/lib/fields.tsxpackages/fields/src/lib/html.tsxpackages/fields/src/lib/measure.tsxpackages/fields/src/lib/multicheck.tsxpackages/fields/src/lib/number.tsxpackages/fields/src/lib/object-select.tsxpackages/fields/src/lib/radio.tsxpackages/fields/src/lib/rangeslider.tsxpackages/fields/src/lib/select.tsxpackages/fields/src/lib/text.tsxpackages/fields/src/lib/textarea.tsxpackages/fields/src/lib/token-select.tsxpackages/icons/src/lib/block-manager.tsxpackages/icons/src/lib/block.tsxpackages/icons/src/lib/check-all.tsxpackages/icons/src/lib/custom-redirect.tsxpackages/icons/src/lib/filter-lines.tsxpackages/icons/src/lib/gears.tsxpackages/icons/src/lib/incognito.tsxpackages/icons/src/lib/license-key.tsxpackages/icons/src/lib/locked-user.tsxpackages/icons/src/lib/mark-colored.tsxpackages/icons/src/lib/mark-light.tsxpackages/icons/src/lib/mark-retro.tsxpackages/icons/src/lib/mark-white.tsxpackages/icons/src/lib/mark.tsxpackages/icons/src/lib/monitor.tsxpackages/icons/src/lib/permissions.tsxpackages/icons/src/lib/protected-message.tsxpackages/icons/src/lib/upgrade.tsxpackages/layout/src/components/app-content/index.tsxpackages/layout/src/components/app-header/index.tsxpackages/layout/src/components/app-layout/index.tsxpackages/layout/src/slots/index.tstsconfig.base.json
✅ Files skipped from review due to trivial changes (89)
- package.json
- packages/components/src/lib/cta-select-control/index.tsx
- packages/components/src/lib/field-panel/index.tsx
- packages/cta-admin/src/components/app-header/index.tsx
- packages/cta-admin/src/components/list-view/header.tsx
- packages/cta-admin/src/components/list/bulk-actions/index.tsx
- packages/cta-admin/src/components/list/options/index.tsx
- packages/cta-admin/src/components/list/quick-actions/index.tsx
- packages/cta-admin/src/registries/list-options/export.tsx
- packages/fields/src/lib/measure.tsx
- packages/cta-editor/src/editor/components/header-actions.tsx
- packages/cta-editor/tsconfig.json
- packages/cta-admin/src/registries/list-bulk-actions/delete.tsx
- packages/cta-admin/src/registries/list-bulk-actions/trash.tsx
- packages/block-editor/package.json
- packages/block-editor/src/formats/popup-trigger/index.tsx
- packages/block-editor/src/formats/popup-trigger/inline.tsx
- packages/block-editor/src/formats/popup-trigger/trigger-popover/popup-trigger-editor.tsx
- packages/components/src/lib/confirm-dialogue/index.tsx
- packages/components/src/lib/controlled-tab-panel/tab-button.tsx
- packages/components/src/lib/device-toggle/index.tsx
- packages/components/src/lib/entity-select-control/index.tsx
- packages/components/src/lib/field-row/index.tsx
- packages/components/src/lib/freeform-control/index.tsx
- packages/components/src/lib/popup-select-control/index.tsx
- packages/components/src/lib/radio-buttons-control/index.tsx
- packages/components/src/lib/text-highlight/index.tsx
- packages/fields/src/lib/textarea.tsx
- packages/fields/src/lib/text.tsx
- packages/fields/src/lib/fields.tsx
- packages/components/src/lib/searchable-multicheck-control/index.tsx
- packages/icons/src/lib/gears.tsx
- packages/components/src/lib/full-screen-panel/index.tsx
- packages/icons/src/lib/mark.tsx
- packages/components/src/lib/url-control/index.tsx
- packages/cta-admin/src/components/list-view/index.tsx
- packages/cta-admin/src/components/list-view/notices.tsx
- packages/cta-admin/src/components/list/filters/index.tsx
- packages/cta-admin/src/components/list/index.tsx
- packages/cta-admin/src/App.tsx
- packages/cta-admin/src/registries/list-bulk-actions/disable.tsx
- packages/cta-admin/src/registries/list-bulk-actions/export.tsx
- packages/cta-admin/src/registries/list-filters/status.tsx
- packages/cta-admin/src/registries/list-filters/type.tsx
- packages/fields/src/lib/checkbox.tsx
- packages/fields/src/lib/date.tsx
- packages/fields/src/lib/multicheck.tsx
- packages/fields/src/lib/radio.tsx
- packages/fields/src/lib/number.tsx
- packages/cta-admin/src/registries/list-options/import.tsx
- packages/fields/src/lib/custom-select.tsx
- packages/cta-editor/src/components/debug-notices.tsx
- packages/fields/src/lib/field.tsx
- packages/cta-editor/package.json
- packages/fields/src/lib/object-select.tsx
- packages/icons/src/lib/check-all.tsx
- packages/core-data/src/call-to-actions/selectors.ts
- packages/cta-admin/src/registries/list-quick-actions/export.tsx
- packages/cta-editor/src/registries/header-actions/status.tsx
- packages/cta-editor/src/editor/components/header-options.tsx
- packages/cta-admin/src/registries/list-quick-actions/edit.tsx
- packages/cta-editor/src/registries/tabs/general.tsx
- packages/cta-editor/src/registries/header-actions/history.tsx
- packages/fields/src/lib/color.tsx
- packages/cta-editor/src/registries/header-options/delete.tsx
- packages/icons/src/lib/block.tsx
- packages/icons/src/lib/block-manager.tsx
- packages/fields/src/lib/token-select.tsx
- packages/fields/src/lib/rangeslider.tsx
- packages/icons/src/lib/locked-user.tsx
- packages/icons/src/lib/mark-colored.tsx
- packages/icons/src/lib/filter-lines.tsx
- packages/icons/src/lib/mark-white.tsx
- packages/icons/src/lib/monitor.tsx
- packages/icons/src/lib/permissions.tsx
- packages/icons/src/lib/custom-redirect.tsx
- packages/icons/src/lib/mark-light.tsx
- packages/icons/src/lib/mark-retro.tsx
- packages/layout/src/components/app-content/index.tsx
- packages/icons/src/lib/upgrade.tsx
- packages/icons/src/lib/protected-message.tsx
- packages/layout/src/components/app-layout/index.tsx
- packages/icons/src/lib/license-key.tsx
- packages/icons/src/lib/incognito.tsx
- packages/block-editor/src/formats/popup-trigger/trigger-popover/popup-trigger-viewer.tsx
- packages/fields/src/lib/select.tsx
- packages/cta-editor/src/editor/hocs/with-modal.tsx
- packages/fields/src/hooks/use-fields.tsx
- packages/cta-admin/src/registries/list-quick-actions/trash.tsx
🚧 Files skipped from review as they are similar to previous changes (2)
- tsconfig.base.json
- packages/core-data/src/call-to-actions/actions.ts
- Add @popup-maker/* to import/no-unresolved ignore. These are pnpm workspace packages that resolve at runtime via tsconfig paths and webpack aliases; eslint-plugin-import can't follow the symlinks in CI even though the code works. - Fix translator comment format on pre-existing sprintf calls so @wordpress/i18n-translator-comments is satisfied. The rule expects '%d: description' / '%s: description' form; several call sites used '1. description' which fails validation. - Rewrite ternary in popup-trigger-viewer.tsx so the translator comment directly precedes sprintf (rule doesn't recognize comments between ? and sprintf as 'preceding').
The @wordpress/i18n-translator-comments rule wants the comment to precede __() specifically, not the outer sprintf() call.
- cta-editor/editor/index.ts: import ComponentType from 'react' to match the other HOC files (with-data-store, with-modal, with- query-params all import from 'react', not @wordpress/element). - fields/lib/html.tsx: document the dangerouslySetInnerHTML contract — content must be sanitized server-side by the REST controller before reaching this component. - core-data: replace unsafe 'as WPNotice[]' casts in getNotices / getNoticeById with a shared isValidNoticeStatus() predicate that filters to known-status notices before casting. Honest narrowing instead of a lying cast; keeps WPNotice's literal status union accurate.
jsdoc/require-param rule requires @param for each function parameter when a JSDoc block exists. Added missing @param tags on the new comments for isValidNoticeStatus and HtmlField.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/cta-editor/src/registries/tabs/general.tsx (1)
131-132: Redundantas stringcast.
SelectControl'sonChangealready providestypeasstring, sotype as stringis a no-op. Safe to drop for clarity, but not blocking.♻️ Proposed tweak
- const ctaType = registeredCtaTypes[ type as string ]; + const ctaType = registeredCtaTypes[ type ];🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/cta-editor/src/registries/tabs/general.tsx` around lines 131 - 132, The code uses an unnecessary TypeScript cast in the onChange handler—remove the redundant "as string" so the onChange parameter `type` is used directly when indexing `registeredCtaTypes` (i.e., change the arrow function passed to SelectControl/onChange to use `registeredCtaTypes[type]` instead of `registeredCtaTypes[type as string]`), keeping the rest of the handler logic intact.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/core-data/src/types/notices.ts`:
- Around line 57-98: The Notice interface is missing the optional __unstableHTML
property which causes TypeScript excess-property errors when callers spread WP
notice options into createNotice; add "__unstableHTML?: boolean" to the Notice
interface definition (the exported Notice type in the notices.ts type
declarations) so the shape matches `@wordpress/notices`' NoticeOptions and callers
can pass that flag without type errors.
---
Nitpick comments:
In `@packages/cta-editor/src/registries/tabs/general.tsx`:
- Around line 131-132: The code uses an unnecessary TypeScript cast in the
onChange handler—remove the redundant "as string" so the onChange parameter
`type` is used directly when indexing `registeredCtaTypes` (i.e., change the
arrow function passed to SelectControl/onChange to use
`registeredCtaTypes[type]` instead of `registeredCtaTypes[type as string]`),
keeping the rest of the handler logic intact.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: f68cda61-0e71-44ff-829c-13ed72f183fd
📒 Files selected for processing (25)
.eslintrc.jspackages/block-editor/src/formats/popup-trigger/trigger-popover/popup-trigger-viewer.tsxpackages/components/src/lib/field-panel/index.tsxpackages/components/src/lib/popup-select-control/index.tsxpackages/components/src/lib/text-highlight/index.tsxpackages/core-data/src/call-to-actions/selectors.tspackages/core-data/src/popups/selectors.tspackages/core-data/src/types/notices.tspackages/cta-admin/src/components/list-view/header.tsxpackages/cta-admin/src/components/list/bulk-actions/index.tsxpackages/cta-admin/src/context.tsxpackages/cta-admin/src/index.tsxpackages/cta-admin/src/registries/list-bulk-actions/delete.tsxpackages/cta-admin/src/registries/list-bulk-actions/disable.tsxpackages/cta-admin/src/registries/list-bulk-actions/enable.tsxpackages/cta-admin/src/registries/list-bulk-actions/trash.tsxpackages/cta-admin/src/registries/list-quick-actions/delete.tsxpackages/cta-admin/src/registries/list-quick-actions/edit.tsxpackages/cta-admin/src/registries/list-quick-actions/trash.tsxpackages/cta-editor/src/editor/index.tspackages/cta-editor/src/registries/tabs/general.tsxpackages/fields/src/hooks/use-fields.tsxpackages/fields/src/lib/field.tsxpackages/fields/src/lib/html.tsxpackages/fields/src/lib/text.tsx
✅ Files skipped from review due to trivial changes (13)
- .eslintrc.js
- packages/cta-admin/src/registries/list-quick-actions/trash.tsx
- packages/cta-admin/src/index.tsx
- packages/cta-admin/src/registries/list-quick-actions/edit.tsx
- packages/cta-admin/src/components/list/bulk-actions/index.tsx
- packages/cta-admin/src/registries/list-bulk-actions/delete.tsx
- packages/components/src/lib/field-panel/index.tsx
- packages/components/src/lib/popup-select-control/index.tsx
- packages/cta-admin/src/registries/list-bulk-actions/trash.tsx
- packages/cta-admin/src/registries/list-quick-actions/delete.tsx
- packages/fields/src/lib/field.tsx
- packages/components/src/lib/text-highlight/index.tsx
- packages/fields/src/hooks/use-fields.tsx
🚧 Files skipped from review as they are similar to previous changes (7)
- packages/cta-editor/src/editor/index.ts
- packages/fields/src/lib/text.tsx
- packages/cta-admin/src/context.tsx
- packages/core-data/src/call-to-actions/selectors.ts
- packages/cta-admin/src/registries/list-bulk-actions/disable.tsx
- packages/block-editor/src/formats/popup-trigger/trigger-popover/popup-trigger-viewer.tsx
- packages/core-data/src/popups/selectors.ts
- notices.ts: add __unstableHTML?: boolean to the Notice (NoticeOptions) interface to match @wordpress/notices upstream. Callers spreading this flag previously got excess-property errors. - general.tsx: drop redundant 'as string' cast on SelectControl's onChange parameter — it's already typed as string by WP's SelectControl.
This was referenced Apr 22, 2026
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
1 participant
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
Fixes the
build:tscstep of theJS/TS Code QualityCI job that's been failing since the CI Summary rollup was added for branch protection (#1207 was the PR that surfaced it). The rollup promoted previously-maskedcontinue-on-error: truefailures to blocker status, exposing three pre-existing TypeScript config issues.None of these are caused by #1207 — they're pre-existing on
developand were only hidden because the old CI setup lettscfailures silently pass.What CI was seeing
From the last failed run on PR #1207 before it was admin-merged:
"esModuleInterop=false"and"moduleResolution=node10"are deprecated in TS 5.x, will be removed in TS 7.types: [...]array in the base tsconfig referenced type libs not reachable from each package's resolution root.build:tscbut expected jest globals the source packages didn't declare.Changes
tsconfig.base.jsontypes: [...]. The hardcoded["node", "react", "jest", "@testing-library/jest-dom"]array forced every package to resolve type libs it didn't have as direct deps. Packages that need globals (jquery, etc.) already declare them in their own tsconfig. Empty array disables TS's auto-inclusion of every hoisted@types/*, which is the correct monorepo behavior.__tests__/and*.test.{ts,tsx}) instead of the unusedtest/. Test files shouldn't participate in type-build.TS5107with"ignoreDeprecations": "5.0". Migrating tomoduleResolution: "bundler"is the long-term fix but breaks type-root resolution under pnpm hoisting in this monorepo; defer that to a dedicated task before TS 7.0 removes the escape hatch.packages/icons/src/lib/*.tsx(18 files)Annotate each exported icon const with
: JSX.Element. OnceTS5107stops blocking, tsc proceeds to emit.d.tsfiles and hitsTS2742because pnpm hoists@types/reactinto a versioned.pnpm/path that can't be named portably. Explicit annotations let tsc emit the type name without a reference to the hoisted path.packages/core-data/src/types/notices.ts+ actions.tsThe local
Noticetype had drifted from@wordpress/notices::NoticeOptionsin three ways that tsc surfaced once the mask was lifted:Notice.statuswas widened with| string, erasing the narrow'error' | 'warning' | 'success' | 'info'union.Notice extended WPNotice, so spreading it intocreateNotice(status, content, options)as the third arg passed extras (status,content,spokenMessage,__unstableHTML) not part of the options contract.NoticeAction.url/.onClicktyped asstring | null/Function | nulldidn't match WP'sstring | undefined/() => void.Action handlers that referenced
Notice['status']/Notice['content']for their input param types now referenceWPNoticedirectly, which carries those fields since they're function arguments rather than options.Note: this is a type-only change to the emitted public types of
@popup-maker/core-data.NoticeAction.urlgoing fromstring | nulltostring | undefinedis a technical breaking change for TS consumers, but runtime semantics are unchanged.pnpm-lock.yamlPost-install pnpm switched a few internal workspace refs from
file:tolink:protocol. No version changes.What's NOT in this PR
Not all
build:tscerrors are fixed. Local runs surface additionalTS2742errors inpackages/components,packages/cta-admin, etc. — but CI doesn't hit those because CI's install layout differs from local, and the cascade only reaches them afterTS5107is silenced. If CI surfaces them after merge, a follow-up PR can annotate those packages the same wayiconswas annotated.Test plan
js-qualityjob reaches green (or reveals a new, distinct class of error)CI Summaryrollup no longer blocks future PRs on the pre-existingTS5107deprecation🤖 Generated with Claude Code
Summary by CodeRabbit
Refactor
Chores