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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

[styled] Implicit styleProps should be explicit #26410

Closed
eps1lon opened this issue May 21, 2021 · 0 comments 路 Fixed by #26461
Closed

[styled] Implicit styleProps should be explicit #26410

eps1lon opened this issue May 21, 2021 · 0 comments 路 Fixed by #26461
Assignees
Labels
new feature New feature or request package: styled-engine Specific to @mui/styled-engine

Comments

@eps1lon
Copy link
Member

eps1lon commented May 21, 2021

Summary 馃挕

styleProps is currently repeating the same mistakes as React.FC with children.
By default it assumes a generic shape that can result in writing code that's impossible at runtime when we should be able to verify it statically

Examples 馃寛

const FancySpan = styled('span')(({ styleProps }) => {
  const size = styleProps?.size;
  if (typeof size !== 'number') throw new TypeError('Expected a number')
  return { height: size + 5 }
});

<FancySpan styleProps={{ size: 1 }} />;

Motivation 馃敠

This currently type-checks when it should fail:

const FancySpan = styled('span')(({ styleProps }) => {
  const size = styleProps?.size;
  if (typeof size !== 'number') throw new TypeError('Expected a number')
  return { height: size + 5 }
});

// currently type-checks but fails at runtime
<FancySpan styleProps={{ size: '' }} />;
const FancyDiv = styled('span')()
// @ts-expect-error There are no styleProps. Passing this is an author error that should be caught
<FancyDiv styleProps={{ }} />;

Note that you can get better type-safety with explicit prop shapes but considering past experience with implicit props this should be default:

const ClockNumberRoot = styled('span', { skipSx: true })<{ styleProps: { inner: boolean }>(
  ({ theme, styleProps }) => ({
....

// @ts-expect-error `theme` is required now
<ClockNumberRoot styleProps={{inner}} />

styleProps should be explicitly declared not implicitly provided considering styled does not inject such a prop.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
new feature New feature or request package: styled-engine Specific to @mui/styled-engine
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants