Update v4 spec about sharing props type

[mununki]

This PR updates the JSX v4 spec about sharing props type regarding the PR #699

[cristianoc]

Would you clarify in the text above what's the role of <'a>. Does the name 'a matter, or is it only used to specify the number of type arguments?

[mununki]

It is just telling the number of type arguments are available. Better to remove it?

[cristianoc]

Not sure I understand: or can be 1 zero type args, or 2 more than zero. And the user needs to specify which one is true?

[mununki]

Sorry for making communication cost. My point was that I expected there were some use cases:

type sp1 = ...

@react.component(:sp1)

type sp2<'a> = ...

@react.component(:sp2<'a>)
// or
@react.component(:sp2<string>)

type sp3<'a, 'b, ...> = ...

@react.component(:sp3<'a, 'b, ...>)
// or
@react.component(:sp3<string, 'b, ...>)

The text is saying that the type argument(s) is available. The name of 'a nor the number of type arguments wouldn't matter.

[cristianoc]

Does it work also if the type definition has zero type arguments?

[mununki]

If you mean this case:

type sp1 = ...

@react.component(:sp1)

Yes, it works. https://github.com/rescript-lang/syntax/blob/master/tests/ppx/react/expected/sharedProps.res.txt#L38

[cristianoc]

I read the source code. So the number of type variables does matter. The name matters only so much as it normally matters, but not less.

So I think what's missing in the doc is one example with 0 and one with 2 type variables.

[cristianoc]

One should also remark on the fact that a type called props cannot be defined in the current module or there will be a type error due to type redefinition. (It could at most be defined outside the current module)

[mununki]

One should also remark on the fact that a type called props cannot be defined in the current module or there will be a type error due to type redefinition. (It could at most be defined outside the current module)

I think this is already noted as https://github.com/rescript-lang/syntax/blob/master/cli/JSXV4.md#transformation-for-component-definition

[mununki]

How does it look? c052cb9 1112ae6

[cristianoc]

@mattdamon108 made some changes: can you take a look?

What's not clear to me is why one does this:

  let make = ({x, y, _}: props<'a, 'b>) => React.string(x ++ y ++ z)

and not this:

  let make = ({x, y, _}: props<_>) => React.string(x ++ y ++ z)

[cristianoc]

Also what's unclear to me is why the feature combines a partial type definition (passing certain arguments), which can e.g. be used with a wrong number of arguments or something. And what's the type error going to look like if used wrong.

Another possibility is having the simples mechanism possible, and just pass the name of the type, and provide zero additional type definitions in the PPX. And let the user fully define that type. Probably requiring that it has at least one type argument (in the instructions for use), so there's no special case to worry about.

[mununki]

@mattdamon108 made some changes: can you take a look?

What's not clear to me is why one does this:

  let make = ({x, y, _}: props<'a, 'b>) => React.string(x ++ y ++ z)

and not this:

  let make = ({x, y, _}: props<_>) => React.string(x ++ y ++ z)

Actually, the feature generates the latter. https://github.com/rescript-lang/syntax/blob/master/tests/ppx/react/expected/sharedProps.res.txt

I misread. Right, you're correct. I'll fix it.

[mununki]

Also what's unclear to me is why the feature combines a partial type definition (passing certain arguments), which can e.g. be used with a wrong number of arguments or something. And what's the type error going to look like if used wrong.

Here is the example to show how it works and my logic:

Case 1. simple case

type sp = ...

@react.component(:sp)
// transformed to
type props = sp

Case 2.

type sp1<'a> = ...

@react.component(:sp1<'a>) // Ptyp_var
// transformed to
type props<'a> = sp<'a> // props<'a>

Case 3.

type sp1<'a> = ...

@react.component(:sp<string>) // Ptyp_constr
// transformed to
type props = sp<string> // just props

I just show the case of having 0 and 1 type argument example, but it would be the same for the 2, 3, ... type arguments. Both cases 2 and 3 are possible as use cases. So, if the core_type in payload of @react.component has type arguments, then I distinguish the arguments between Ptyp_var or else. This is why it makes the partial type definition.

Another possibility is having the simples mechanism possible, and just pass the name of the type, and provide zero additional type definitions in the PPX. And let the user fully define that type. Probably requiring that it has at least one type argument (in the instructions for use), so there's no special case to worry about.

Yes, I agree with your point. That would make more clean implementation as well as a clear user guide.
IMHO, I guess the current logic would not make many problems in the userland. If the user encounters the type error caused by a mismatched number of type arguments, then would the user rather define the type fully?

[cristianoc]

I think the tradeoffs in terms of implementations and uses are clear. Up to you to choose which one looks best.

[mununki]

I've made a PR to fix and update the spec accordingly.

[cristianoc]

Ready to merge?

[mununki]

Ready to merge?

Yes!