More generalized props type for ref and type variables

[mununki]

This PR has two purposes.

1. The props type with unbounded type variables

Fix the issue https://forum.rescript-lang.org/t/call-for-help-test-the-react-jsx-v4/3713/57?u=moondaddi, which is how to handle type variables for making props type.

type t<'a>

// original
external make: (~ref: t<'a>) => React.element = "..."

// converted to
// before
type props = {ref: t<'a>} // Unbound type parameter 'a
external make: React.componentLike<props, React.element> = "..."

// after
type props<'ref> = {ref: 'ref}
external make: React.componentLike<props<t<'a>>, React.element> = "..."
}

2. More generalized type for ref

The above issue reminds me that the constraint of type ref. The type constraint ReactDOM.Ref.currentDomRef is not flexible in case the JSX ppx is used with other jsx libraries, such as React Native. The type of ref would be different in each library.

Without type annotation for ref, ReactDOM.Ref.currentDomRef as default

// original
@react.component React.forwardRef((~x, ref) => body)

// before
type props<'x> = {
  ref?: ReactDOM.Ref.currentDomRef,
  x: x,
}

({x, _}: props<'x>, ref) => body

// after
type props<'x, 'ref> = {
  x: 'x,
  ref?: 'ref,
}

({x, _}: props<'x, ReactRef.currentDomRef>, ref) => body

With type annotation

// original
@react.component React.forwardRef((~x, ref: Js.Nullable.t<ReactNative.Ref.t>) => body)

// after
type props<'x, 'ref> = {
  x: 'x,
  ref?: 'ref,
}

({x, _}: props<'x, ReactNative.Ref.t>, ref: Js.Nullable.t<ReactNative.Ref.t>) => {
  let _ = ref->Js.Nullable.toOption->Belt.Option.map(ReactNative.Ref.domRef)
}

Js.Nullable.t<'ref> is used for the runtime safety instead of option<'ref>, because the value of ref could be null. 'ref should be used in the props type for calling from the other application sites.

[cristianoc]

Would you rebase past this? #638
Or re-create in case the changes are too big.

[cristianoc]

@mattdamon108
For 1, what's the idea?
It looks like in the original code there was a constraint on type t. Presumably the user intended to have that constraint. Is that completely gone now?

[cristianoc]

@mattdamon108
For 1, what's the idea?
It looks like in the original code there was a constraint on type t. Presumably the user intended to have that constraint. Is that completely gone now?

Sorry I did not see it's there. Ignore me.
React.componentLike<props<t<'a>>

[mununki]

Would you rebase past this? #638 Or re-create in case the changes are too big.

Done!

[cristianoc]

Looks great.
Do all the examples compile fine?

Perhaps 1 more line of comment on where the removed Js.Nullable.t is coming from (2nd arg of forwardRef).
Should Js.nullable be removed in addition of Js.Nullable.t, in case some details of the bindings change in future?

[mununki]

Do all the examples compile fine?

It is fine in the example project. I’m going to run more in my company project. Let you know the result.

Perhaps 1 more line of comment on where the removed Js.Nullable.t is coming from (2nd arg of forwardRef).
Should Js.nullable be removed in addition of Js.Nullable.t, in case some details of the bindings change in future?

Can I ask what you mean the removed Js.Nullable.t? Is it going to be removed?

[cristianoc]

It is fine in the example project. I’m going to run more in my company project. Let you know the result.

Great. I'll merge assuming there are no issues. If there are, we can revisit.

Can I ask what you mean the removed Js.Nullable.t? Is it going to be removed?

Sorry I meant function stripJsNullable.

[mununki]

Sorry I meant function stripJsNullable.

Ah! It’s little tricky part, though. If the 2nd arg of forwardRef can be sure not a null value, we can use the option type, instead of Js.Nullable.t.

Not a very clear picture how to improve this part now, but we can tackle this part later to improve more cleaner and less magic.