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

Allow optional DataGrid props to be null #2406

Merged
merged 3 commits into from
May 14, 2021
Merged

Allow optional DataGrid props to be null #2406

merged 3 commits into from
May 14, 2021

Conversation

nstepien
Copy link
Contributor

@nstepien nstepien commented May 14, 2021

Allowing null for optional props means this pattern won't be necessary anymore:

<DataGrid optionalProp={maybeNull ?? undefined} />

??, ??=, ?., and != null makes this trivial thankfully.
Only downside I found was with default function params, param = default doesn't replace nulls.
Non-ts users should get fewer surprises as well.

I've also fixed some missed spots with the new K generic.

Column props, and maybe other props will be done in separate PRs.

@nstepien nstepien self-assigned this May 14, 2021
rowHeight ??= 35;
headerRowHeight ??= typeof rowHeight === 'number' ? rowHeight : 35;
headerFiltersHeight ??= 45;
const summaryRowHeight = rawSummaryRowHeight ?? (typeof rowHeight === 'number' ? rowHeight : 35);
Copy link
Contributor Author

@nstepien nstepien May 14, 2021

Choose a reason for hiding this comment

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

There is a little bit of nuance here vs default params, if a param isn't defaulted in the parameter list, then it keeps its original type in functions, as TS cannot know that those functions are only called after the ??= line. I wish we could use ??= in the param list. It also means they can be re-assigned to null or undefined if we're not careful.

function fnA(param = 1) {
  param; // type; number

  function test() {
    param; // type: number
  }
}

function fnB(param: number | undefined | null) {
  param; // type: number | undefined | null
  param ??= 1;
  param; // type: number

  function test() {
    param; // type: number | undefined | null
    // this is because we could call `test` before the line where we set it to 1, or it can be reassigned to `null | undefined`
  }
}

@nstepien nstepien marked this pull request as ready for review May 14, 2021 11:24
@nstepien nstepien requested a review from amanmahajan7 May 14, 2021 11:24
@nstepien nstepien merged commit 8b1660e into canary May 14, 2021
@nstepien nstepien deleted the null branch May 14, 2021 12:23
This was referenced May 14, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants