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

[RadioGroup] Update useRadioGroup.d.ts #19001

Merged
merged 1 commit into from
Jan 8, 2020
Merged

[RadioGroup] Update useRadioGroup.d.ts #19001

merged 1 commit into from
Jan 8, 2020

Conversation

NMinhNguyen
Copy link
Contributor

@NMinhNguyen NMinhNguyen commented Dec 27, 2019

Remove unnecessary export {}.

This is a follow-up to PR #18920 (sorry, I forgot to remove this when I inlined ContextFromPropsKey).

Remove unnecessary `export {}`.

This is a follow-up to PR #18920.
@mui-pr-bot
Copy link

No bundle size changes comparing d57c223...d14242f

Generated by 🚫 dangerJS against d14242f

@oliviertassinari
Copy link
Member

I have noticed that we use this approach in a couple of other places. Are they all justified?

@NMinhNguyen
Copy link
Contributor Author

Are they all justified?

@oliviertassinari I think they are because they occur in files that have a local type defined but we may not want to export it as part of the public API.

@NMinhNguyen
Copy link
Contributor Author

@oliviertassinari upon further inspection, there's https://github.com/mui-org/material-ui/blob/fb0ab9d667a911459a47a66e64d2e27ec7185bc3/packages/material-ui-types/index.d.ts#L3-L4 that uses export {} but doesn't declare any local types, although it probably has a higher likelihood of declaring such types in the future.

Lastly, there's https://github.com/mui-org/material-ui/blob/fb0ab9d667a911459a47a66e64d2e27ec7185bc3/packages/material-ui/src/FormControl/useFormControl.d.ts#L4-L5 where @eps1lon said #18920 (comment)

I guess I could've inlined these keys to avoid having to name things.

I should've done this, good point 👍 Where those come from is irrelevant for the context and types.

so I could add that change to this PR? If I touch more than 1 component, is the commit scope [Core]? Or should I create a separate commit for [FormControl]?

@oliviertassinari
Copy link
Member

@NMinhNguyen Thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: radio This is the name of the generic UI component, not the React module! typescript
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants