Skip to content
This repository was archived by the owner on Jun 15, 2023. It is now read-only.

Type error on empty record with fragments in automatic mode #703

Closed
cristianoc opened this issue Oct 26, 2022 · 8 comments · Fixed by rescript-lang/rescript-react#77
Closed
Assignees

Comments

@cristianoc
Copy link
Contributor

This:

module Automatic = {
  @react.component
  let make = () => {
    <> </>
  }
}

produces:

module Automatic = {
  type props = {}

  @react.component
  let make = (_: props) => {
    React.jsx(React.jsxFragment, {children: {}})
  }
  let make = {
    let \"OptionalKeyType$Automatic" = props => make(props)

    \"OptionalKeyType$Automatic"
  }
}

which gives a type errow (without location information):

  We've found a bug for you!
  /Users/cristianocalcagno/GitHub/rescript-vscode/analysis/tests/src/Completion.res

  Empty record literal {} should be type annotated or used in a record context.
@mununki
Copy link
Member

mununki commented Oct 26, 2022

I'll look into it

@cristianoc
Copy link
Contributor Author

Probably one need to add a type annotation just like in other cases : {} : props<_>

@mununki
Copy link
Member

mununki commented Oct 26, 2022

babel output

I think it should be:

React.jsx(React.jsxFragment, {}: props<_>)

@mununki
Copy link
Member

mununki commented Oct 26, 2022

Or, in rescript-react

// React.res
type fragmentProps<'children> = {children?: 'children} // ? optional

then

React.jsx(React.jsxFragment, {})

It works fine

@cristianoc
Copy link
Contributor Author

Or, in rescript-react

// React.res
type fragmentProps<'children> = {children?: 'children} // ? optional

then

React.jsx(React.jsxFragment, {})

It works fine

This looks cleaner.

@mununki
Copy link
Member

mununki commented Oct 26, 2022

I'll make a PR soon.

@cristianoc
Copy link
Contributor Author

The one thing to check is, that when children is actually present, the compiler understands it's a record type inside, and does not add any of that runtime magic for optionals.

@mununki
Copy link
Member

mununki commented Oct 26, 2022

The one thing to check is, that when children is actually present, the compiler understands it's a record type inside, and does not add any of that runtime magic for optionals.

If I understand correctly, isn't it same to the uppercase and lowercase component too? Z.props would have children props as optional and Jsx.domProps has an optional children field already.

<Z /> // jsx(Z.make, {})
<Z> ... </Z> // jsx(Z.make, {children: ...})

<div /> // jsx("div", {})
<div> ... </div> // jsx("div", {children: ... })

I think the current type fragmentProps is wrong. The children field should be optional as we discussed, because the fragment could have children or not.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants