Skip to content

[internal] Fix TypeScript 6 validation#4420

Open
atomiks wants to merge 6 commits intomui:masterfrom
atomiks:codex/typescript-6-compat
Open

[internal] Fix TypeScript 6 validation#4420
atomiks wants to merge 6 commits intomui:masterfrom
atomiks:codex/typescript-6-compat

Conversation

@atomiks
Copy link
Contributor

@atomiks atomiks commented Mar 24, 2026

This fixes the current TypeScript validation failures that show up with the repo's current toolchain.

The main change is small: FocusOptions is augmented in the shared React globals so the existing focus({ focusVisible: true }) calls typecheck again with the DOM typings currently used in this repo. The Vite playground also stops using the deprecated baseUrl option.

There is one small docs source change as well: the CSP Provider page now avoids raw tag names in its description so docs validation and Vale stay happy under the current docs pipeline.

Changes

  • Add focusVisible to the shared FocusOptions typing in packages/react/src/global.d.ts.
  • Remove the stale call-site suppression workaround from the label and select code paths.
  • Remove the deprecated baseUrl option from the Vite playground tsconfig.
  • Tweak the CSP Provider docs description so generated docs validation stays clean.

@atomiks atomiks added docs Improvements or additions to the documentation. type: bug It doesn't behave as expected. typescript labels Mar 24, 2026
@pkg-pr-new
Copy link

pkg-pr-new bot commented Mar 24, 2026

commit: a8fd202

@mui-bot
Copy link

mui-bot commented Mar 24, 2026

Bundle size report

Bundle Parsed size Gzip size
@base-ui/react 0B(0.00%) 0B(0.00%)

Details of bundle changes


Check out the code infra dashboard for more information about this PR.

@netlify
Copy link

netlify bot commented Mar 24, 2026

Deploy Preview for base-ui ready!

Name Link
🔨 Latest commit a8fd202
🔍 Latest deploy log https://app.netlify.com/projects/base-ui/deploys/69c1e179ac442600082c458f
😎 Deploy Preview https://deploy-preview-4420--base-ui.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

@atomiks atomiks requested a review from dav-is March 24, 2026 00:45
@atomiks atomiks marked this pull request as ready for review March 24, 2026 01:02
@atomiks atomiks changed the title [all components] Fix TypeScript 6 validation [internal] Fix TypeScript 6 validation Mar 24, 2026
Copy link
Member

@LukasTy LukasTy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. 👌

@michaldudak
Copy link
Member

Codex Review

Overview

This PR fixes the current TypeScript 6 validation fallout by centralizing the FocusOptions.focusVisible typing, removing the temporary per-call suppressions that depended on it, cleaning up the Vite playground tsconfig, and updating the CSP Provider docs metadata so the docs pipeline stays clean.

Findings (None)

No blocking issues found in this patch.

Confidence: 4/5

High confidence based on a full base...head pass over the five touched files, plus targeted checks against repository guidance, nearby history/blame, prior PR context, and the current PR discussion.

@LukasTy
Copy link
Member

LukasTy commented Mar 24, 2026

PR Review: [internal] Fix TypeScript 6 validation — #4420

Author: @atomiks | Branch: codex/typescript-6-compatmaster
Size: +8 / −4 lines across ~5 files | Labels: docs, type: bug, typescript


Summary of Changes

File Change
packages/react/src/global.d.ts Augments the global FocusOptions interface to add an optional focusVisible property. This is the central fix.
packages/react/src/labelable-provider/useLabel.ts Removes the now-unnecessary // @ts-expect-error not available in types yet comment on a focusVisible: true call.
packages/react/src/select/root/SelectRoot.tsx Same — removes an analogous // @ts-expect-error - focusVisible is not yet in the lib.dom.d.ts suppression.
playground/vite-app/tsconfig.app.json Removes deprecated "baseUrl": "." option (the paths config keeps working without it in modern TS).
docs/src/app/(docs)/react/utils/csp-provider/page.mdx Replaces raw <style> / <script> HTML tags in the content field with plain-text "style" / "script", fixing docs-pipeline validation and Vale.

Analysis

Correctness

  • The FocusOptions augmentation is well-scoped: it lives in global.d.ts, uses a proper declare global block, and only adds the single focusVisible property that's already supported at runtime by modern browsers. The boolean | undefined type is correct.
  • Removing the two @ts-expect-error suppressions is the right follow-up — with the augmentation, TS no longer errors on those call-sites, so keeping the directives would itself cause TS errors (unused @ts-expect-error).
  • Removing "baseUrl": "." from the playground tsconfig is safe; paths continues to work because module resolution already uses the tsconfig's directory as the implicit base.

Docs fix

  • Replacing literal <style> and <script> in the MDX meta content with plain words avoids the docs pipeline interpreting them as JSX tags. Straightforward and correct.

Potential concerns

  • None significant. The augmentation is additive and non-breaking. It doesn't conflict with future TS lib updates — once TS ships official focusVisible in lib.dom.d.ts, the augmentation will simply merge harmlessly (identical member, same type).
  • Bundle size impact is 0 B (confirmed by CI).

CI status: ✅ 23/23 checks pass
Existing review: @LukasTy approved ("LGTM")


Merge Readiness Score: 9.5 / 10

Dimension Score Notes
Correctness 10 All changes are mechanically sound
Risk 10 Purely type-level + docs metadata; zero runtime impact
Scope 10 Minimal, focused diff (8 additions, 4 deletions)
Testing/CI 9 All CI green; no new tests, but none are needed for type augmentations
Review coverage 9 1 approval; still awaiting 4 requested reviewers

Verdict: Ready to merge. The only reason it's not a full 10 is that several requested reviewers (@dav-is, @colmtuite, @michaldudak, @flaviendelangle) haven't weighed in yet, which is a process consideration rather than a code concern. The changes themselves are safe, minimal, and correct.

Completed: Present summary and merge readiness (4/4)

<Meta
name="description"
content="A CSP provider component that applies a nonce to inline <style> and <script> tags rendered by Base UI components, and can disable inline <style> elements."
content="A CSP provider component that applies a nonce to inline style and script tags rendered by Base UI components, and can disable inline style elements."
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should keep the non-breaking space

Suggested change
content="A CSP provider component that applies a nonce to inline style and script tags rendered by Base UI components, and can disable inline style elements."
content="A CSP provider component that applies a nonce to inline style and script tags rendered by Base UI components, and can disable inline style elements."

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

docs Improvements or additions to the documentation. type: bug It doesn't behave as expected. typescript

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants