Skip to content

[all components] Fix type portability #2912

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 26 commits into from
Oct 8, 2025
Merged

[all components] Fix type portability #2912

merged 26 commits into from
Oct 8, 2025

Conversation

@atomiks
Copy link
Contributor

@atomiks atomiks commented Oct 3, 2025 •
edited
Loading

Fixes #2881
Fixes #2629

@Janpot, I created a few test files in test/public-types to ensure the types are portable. Maybe it should run in CI?

With this change, types are visible in Intellisense:

Screenshot 2025-10-07 at 11 12 32 pm

The main import always comes first so pressing Enter will still be correct, at which point you can ignore the top-level types and just use namespaces

Screenshot 2025-10-07 at 11 13 29 pm

@atomiks atomiks added type: bug It doesn't behave as expected. typescript scope: all components Widespread work has an impact on almost all components. labels Oct 3, 2025
@pkg-pr-new
Copy link

pkg-pr-new bot commented Oct 3, 2025 •
edited
Loading

vite-css-base-ui-example

pnpm add https://pkg.pr.new/mui/base-ui/@base-ui-components/react@2912

pnpm add https://pkg.pr.new/mui/base-ui/@base-ui-components/utils@2912

commit: 383cad3

@netlify
Copy link

netlify bot commented Oct 3, 2025 •
edited
Loading

✅ Deploy Preview for base-ui ready!

Name Link
🔨 Latest commit 383cad3
🔍 Latest deploy log https://app.netlify.com/projects/base-ui/deploys/68e64e13f368660008fcf203
😎 Deploy Preview https://deploy-preview-2912--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.

@mui-bot
Copy link

mui-bot commented Oct 3, 2025 •
edited
Loading

Bundle size report

Bundle Parsed size Gzip size
@base-ui-components/react 🔺+4B(0.00%) ▼-1B(0.00%)

Details of bundle changes

@github-actions github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged. label Oct 6, 2025
@atomiks
Merge branch 'master' into fix/type-portability-2
7d9b5ed 
Signed-off-by: atomiks <cc.glows@gmail.com>
@github-actions github-actions bot removed the PR: out-of-date The pull request has merge conflicts and can't be merged. label Oct 7, 2025
@github-actions github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged. label Oct 7, 2025
@github-actions github-actions bot removed the PR: out-of-date The pull request has merge conflicts and can't be merged. label Oct 7, 2025
atomiks
atomiks commented Oct 7, 2025
View reviewed changes
test/public-types/index.tsx
Comment on lines +24 to +26
export const MyCheckbox = React.forwardRef<HTMLDivElement, CheckboxProps>(() => {
return <div />;
});
Copy link
Contributor Author

@atomiks atomiks Oct 7, 2025

Choose a reason for hiding this comment

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

The main public test for type portability

atomiks
atomiks commented Oct 7, 2025
View reviewed changes
packages/react/src/dialog/index.ts
Comment on lines +2 to +10

export type * from './root/DialogRoot';
export type * from './trigger/DialogTrigger';
export type * from './portal/DialogPortal';
export type * from './popup/DialogPopup';
export type * from './backdrop/DialogBackdrop';
export type * from './title/DialogTitle';
export type * from './description/DialogDescription';
export type * from './close/DialogClose';
Copy link
Contributor Author

@atomiks atomiks Oct 7, 2025

Choose a reason for hiding this comment

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

Makes types portable (re-exports top-level types). Namespaces themselves re-export those top-level types so there are no breaking changes

@atomiks atomiks marked this pull request as ready for review October 7, 2025 12:15
@atomiks atomiks requested a review from Janpot October 7, 2025 12:24
@Janpot
Copy link
Member

Janpot commented Oct 7, 2025

Maybe it should run in CI?

Yes absolutely. That's the reason I named the script release:test (as opposed to test) so it could be set up as a command that runs on all workspaces that runs after release:build has run. the test command doesn't necessarily depend on production build.

Janpot
Janpot reviewed Oct 7, 2025
View reviewed changes
.github/workflows/ci.yml Outdated
- run: pnpm install
- run: pnpm release:build
- run: pnpm validate-declarations
- run: pnpm -F ./test/public-types release:test
Copy link
Member

@Janpot Janpot Oct 7, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

🤔 This and validate-declarations kind of feels more at place in circleci.

Not that it matters that much

@atomiks atomiks force-pushed the fix/type-portability-2 branch 3 times, most recently from 97f4dda to eed9d75 Compare October 8, 2025 04:56
@atomiks atomiks force-pushed the fix/type-portability-2 branch from eed9d75 to 21d20de Compare October 8, 2025 05:14
@atomiks atomiks requested a review from Janpot October 8, 2025 05:14
Janpot
Janpot reviewed Oct 8, 2025
View reviewed changes
Janpot
Janpot reviewed Oct 8, 2025
View reviewed changes
Copy link
Member

@Janpot Janpot left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Only sampled a few of the library files for review, but seems solid from code infra POV. Leaving final review to Base UI team.

@atomiks @Janpot
Update .circleci/config.yml
69732a5 
Co-authored-by: Jan Potoms <2109932+Janpot@users.noreply.github.com>
Signed-off-by: atomiks <cc.glows@gmail.com>
@github-actions github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged. label Oct 8, 2025
@atomiks
Merge branch 'master' into fix/type-portability-2
deb3e55 
Signed-off-by: atomiks <cc.glows@gmail.com>
@github-actions github-actions bot removed the PR: out-of-date The pull request has merge conflicts and can't be merged. label Oct 8, 2025
@atomiks atomiks force-pushed the fix/type-portability-2 branch from 6d09caa to f2d6a12 Compare October 8, 2025 11:20
mnajdova
mnajdova reviewed Oct 8, 2025
View reviewed changes
Copy link
Member

@mnajdova mnajdova left a comment

Choose a reason for hiding this comment

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

Half way there, will continue but you can keep checking the comments so far.

mnajdova
mnajdova reviewed Oct 8, 2025
View reviewed changes
mnajdova
mnajdova approved these changes Oct 8, 2025
View reviewed changes
Copy link
Member

@mnajdova mnajdova left a comment

Choose a reason for hiding this comment

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

Nice!

@atomiks atomiks merged commit aef03db into mui:master Oct 8, 2025
20 checks passed
@atomiks atomiks deleted the fix/type-portability-2 branch October 8, 2025 12:13
@Janpot Janpot mentioned this pull request Oct 15, 2025
Closed
@michaldudak michaldudak mentioned this pull request Nov 17, 2025
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Janpot Janpot Janpot left review comments

@mnajdova mnajdova mnajdova approved these changes

@michaldudak michaldudak Awaiting requested review from michaldudak michaldudak is a code owner

@colmtuite colmtuite Awaiting requested review from colmtuite colmtuite is a code owner

Assignees

No one assigned

Labels

scope: all components Widespread work has an impact on almost all components. type: bug It doesn't behave as expected. typescript

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Types are not portable Autocomplete.useFilter bundle dts generation error

4 participants

@atomiks @mui-bot @Janpot @mnajdova