Skip to content

Allow function type in untagged variants #6278

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
fhammerschmidt opened this issue May 31, 2023 · 7 comments · Fixed by #6279
Closed

Allow function type in untagged variants #6278

fhammerschmidt opened this issue May 31, 2023 · 7 comments · Fixed by #6279
Milestone

Comments

@fhammerschmidt
Copy link
Member

fhammerschmidt commented May 31, 2023

Just tried to bind the MUI SxProps with untagged variants.

This is the TS definition:

export type SxProps<Theme extends object = {}> =
  | SystemStyleObject<Theme>
  | ((theme: Theme) => SystemStyleObject<Theme>)
  | ReadonlyArray<
      boolean | SystemStyleObject<Theme> | ((theme: Theme) => SystemStyleObject<Theme>)
    >;

and this is my attempt in ReScript:

module SxProps = {
  type theme = {primaryColor?: string}
  type systemStyleObject = {mr?: int, color?: string}

  @unboxed
  type rec t =
    | @as(false) False
    | @as(true) True
    | Object(systemStyleObject)
    | Array(array<t>)
    | Callback(callback)
  and callback = theme => systemStyleObject
}

which, aside from the fact that it's simplified because the array should not be nested and the booleans should only be nested, does not work, because one cannot use a function (the Callback variant in this example) type here.

Would it be possible to add that? Since you can check that easily with typeof == "function".

@cristianoc
Copy link
Collaborator

Do you have references to example TS code that uses that?
Just curious, not important. The description is already clear.

@cristianoc cristianoc added this to the v12 milestone May 31, 2023
@fhammerschmidt
Copy link
Member Author

fhammerschmidt commented May 31, 2023

@cristianoc
Copy link
Collaborator

Yes, here: https://mui.com/system/getting-started/the-sx-prop/#callback-values

Thanks, so it looks like a case where one wants to construct values rather than doing pattern matching on.
While the internal implementation of material UI is the one that does "pattern matching", in the sense of detecting different shapes used.

@fhammerschmidt
Copy link
Member Author

That is true. Looks like in this case a bunch of externals with %identity is still the way to go.

However, I still wonder if it would be a useful addition in other cases.

@zth
Copy link
Collaborator

zth commented May 31, 2023

This pattern, providing either an object or a function to produce that object, is quite common in TS I'd say.

@cknitt
Copy link
Member

cknitt commented May 31, 2023

    | Bool(bool)

instead of

    | @as(false) False
    | @as(true) True

would be nice, too. (Wasn't there a discussion about that somewhere else? Can't find the issue.)

cristianoc added a commit that referenced this issue May 31, 2023
This was done at speed: need to double check that there are no corner cases missing.

Fixes #6278
@zth
Copy link
Collaborator

zth commented May 31, 2023

    | Bool(bool)

instead of

    | @as(false) False
    | @as(true) True

would be nice, too. (Wasn't there a discussion about that somewhere else? Can't find the issue.)

Here: #6231

cristianoc added a commit that referenced this issue Jun 1, 2023
* Add support for functions in untagged variants.

This was done at speed: need to double check that there are no corner cases missing.

Fixes #6278

* test gentype

* Add parens as required for TS case with function type.

* Update CHANGELOG.md
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants