Skip to content
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.

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

[utils] Add typings for @mui-material/styles/cssUtils #29621

Merged
merged 3 commits into from
Dec 2, 2021

Conversation

Semigradsky
Copy link
Contributor

Fixed #29521

@mui-pr-bot
Copy link

mui-pr-bot commented Nov 11, 2021

No bundle size changes

Generated by 🚫 dangerJS against 1fc88e3

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.

This is not a public API, that's why we don't have types for them. I would even remove the unstable_* exports to be honest.

@Semigradsky
Copy link
Contributor Author

Whats about usecase described in #29521? Maybe transformOrigin.horizontal/transformOrigin.vertical should accept string? In this case I will no need toUnitless.

@mnajdova
Copy link
Member

Whats about usecase described in #29521? Maybe transformOrigin.horizontal/transformOrigin.vertical should accept string? In this case I will no need toUnitless.

Ok, I see your point. Let's add the types, but we also need to update the index.d.ts file to export these. Currently they are exported only from index.js. I would export again only the types for the utils that were available before:

export { getUnit as unstable_getUnit, toUnitless as unstable_toUnitless } from './cssUtils';

@github-actions github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged label Nov 26, 2021
@github-actions github-actions bot removed the PR: out-of-date The pull request has merge conflicts and can't be merged label Nov 30, 2021
Copy link
Member

@hbjORbj hbjORbj left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks for your contribution!

@Semigradsky Semigradsky requested a review from mnajdova December 1, 2021 04:55
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.

Sorry for the delay. Looks good thanks. It's a great first pull request on MUI 👌

@hbjORbj hbjORbj merged commit 8e5a7a9 into mui:master Dec 2, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add typings for @mui-material/styles/cssUtils
5 participants