Skip to content

Why don't use optional? in props params when interface work with defaultProps ? #30251

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

Closed
tsaohucn opened this issue Mar 7, 2019 · 3 comments
Labels
Question An issue which isn't directly actionable in code

Comments

@tsaohucn
Copy link

tsaohucn commented Mar 7, 2019

TypeScript Version: 3.3.3333

Question

My team struggle and confuse in problem of should we use optional? when component used defaultProps, we discuss in conclusion that we should use if we consider literary,because you use
defaultProps and that mean you allow user optional, but after I research documents, I found out that's wrong, it seems like you should never use optional? marks in params which have defaultProps

My Question one

Is that correct behavior from my final research result?

My Question two

If my research result is correct, what's reason for implementing that, because it looks like different behavior between nomal case like pure function params, and I tried to serach all kind of reason history document, there's no formal document for describing why TS teams implement that to this API style, I only can guess and filter answer by some git issue and merge history discussion, I prefer to there's a fomal document to describe why implement that to this API style and what different between normal case for reduce confuse from typescript newer .

import React from "react"

interface Props {
  name: string; // no need ?, if add ? defaultProps will fail
}

const Test = ({name}: Props) => {
  return <div>Hello ${name.toUpperCase()}!</div>;
}


const defaultProps = { name: 'aa'};
Test.defaultProps = defaultProps;

export default Test
@basarat
Copy link
Contributor

basarat commented Mar 7, 2019

Before TypeScript added support

You could mark it as optional and TypeScript would be none the wiser. But it forced you to add a non-null assertion e.g.

export interface GreetingProps {
  name?: string
}

export class Greeting extends React.Component<GreetingProps> {
  static defaultProps = {
    name: 'stranger'
  }

  render() {
    // Notice the non-null assertion!
    //                           | |
    //                           | |
    //                           \ /
    //                            V
    return (
      <div>Hello, {this.props.name!.toUpperCase()}</div>
    )
  }
}

So

The whole objective of defaultProps support is to make the props inside the component be a true reflection of the fact that they will be provided.

More

Example taken from : #23812 🌹

@RyanCavanaugh RyanCavanaugh added the Question An issue which isn't directly actionable in code label Mar 7, 2019
@ghost
Copy link

ghost commented Mar 10, 2019

Had the same problem !

Attempt 1

type Props = {
  foo?: string;
  bar: string;
};

const defaultProps = { foo: 'foo!' }

export default class FooBar extends Component<Props> {
  public static defaultProps = defaultProps;
  public render() {
    const foo = this.props.foo; // 💥 string | undefined
    const fooPlz = this.props.foo!; // ✅ string
    const bar = this.props.bar; // ✅ string
    return <Text>{`${foo} ${bar}`}</Text>;
  }
}

Result

typescript doesn't understand that foo is not undefined, you have to use non-null assertion operator

Attempt 2

type Props = {
  foo?: string;
  bar: string;
} & typeof defaultProps; // <-- give the type of const defaultProps

const defaultProps = { foo: 'foo!' }

export default class FooBar extends Component<Props> {
  public static defaultProps = defaultProps;
  public render() {
    const foo = this.props.foo; // ✅ string
    const bar = this.props.bar; // ✅ string
    return <Text>{`${foo} ${bar}`}</Text>;
  }
}

Result

Works well, we use the type of defaultProps to define the final Props.

Caveat

defaultProps is not 'typo-safe'

const defaultProps = { food: 'yummy!' } // no warning

Attempt 3: define defaultProps based on Props

type Props = {
  foo?: string;
  bar: string;
};

// define the type based on Props's properties
type defaultProps = Required<Pick<Props, 'foo'>>;

// implement the type
const defaultPropsError: defaultProps = {
  food: 'yummy!' // 💥 error
};
const defaultProps: defaultProps = {
  foo: 'foo!' // ✅ good
};
// Note: you can also use Object.freeze:
const defaultPropsFreezed = Object.freeze<defaultProps>({
  foo: 'foo!'
})

class FooBar extends Component<Props & defaultProps> {
  //..
  const foo = this.props.foo; // ✅ string
  const bar = this.props.bar; // ✅ string
  //..
}

Result: better and typo-safe

Why does this work

Our component now think that our props is Props & defaultProps so:

Props & defaultProps
=
{ foo?: string; bar: string; } & { foo: string }
=
{ foo: string; bar: string }

Thanks to typescript 3.0 that support defaultProps (cf What's new in TypeScript)

// ✅ no errors
<FooBar bar={'bar'} foo={'foo'} />
// ✅ no errors
<FooBar bar={'bar'} />

Caveat

React.ComponentProps is not friendly anymore (it's expected)

type FooBarProps = React.ComponentProps<typeof FooBar>;
const props: FooBarProps = {
  bar: 'hey' // 💥 Property 'foo' is missing
};

Attempt 4

type Props = {
  foo?: string;
  bar: string;
};

type defaultProps = Required<Pick<Props, 'foo'>>;
const defaultProps = Object.freeze<defaultProps>({
  foo: 'foo!'
})

export default class FooBar extends Component<Props> {
  public static defaultProps = defaultProps;
  private get dProps() {
    return this.props as Props & defaultProps;
  }
  public render() {
    const foo = this.dProps.foo; // ✅ string
    const bar = this.dProps.bar; // ✅ string
    return <Text>{`${foo} ${bar}`}</Text>;
  }
}

//..

// ✅ no errors
<FooBar bar={'bar'} foo={'foo'} />
// ✅ no errors
<FooBar bar={'bar'} />

//..

type FooBarProps = React.ComponentProps<typeof FooBar>;
const props: FooBarProps = {
  bar: 'hey' // ✅ foo is not a problem
};

Result: A little hacky but works well with all my cases.

@matyasf
Copy link

matyasf commented Aug 16, 2021

@TheryFouchter note that attempt number 2 is not React.ComponentProps friendly either

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Question An issue which isn't directly actionable in code
Projects
None yet
Development

No branches or pull requests

5 participants