Skip to content

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

scope: radio Changes related to the radio. typescript

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants