Polyvariant attributes: to bump or not to bump

[dodomorandi]

First of all, thank you for this project. I think that it should be mentioned in reason-react repo and website, your work deserves much more attention from outside 😊 .

Said that, I was interested to open a PR for something I wanted for a long time: support for polyvariant attributes. However, just a few days ago rescript 9.0 came out, and because its intrinsic major breaking change, they decided to make the syntax clearer.
The question is: should I make a PR for the 8.3 version using the older syntax or should I bump bs-platform to 9 and use the new syntax?
Obiously, if you intend to postpone this feature for future releases, just close the issue for now without worries.
Thanks again for your work!

[ryyppy]

not sure what you are asking here. What parts of rescript/react do you want to bump to polyvariants?

[dodomorandi]

I am going to use an example to make things more clear.
Take the very first commented property in domProps:

/* [@optional] [@as "aria-current"] ariaCurrent: page|step|location|date|time|true|false, */

This could not be expressed using Reason syntax, with Rescript it is possible.

This is how I would write in with bs-platform 8.3:

@optional @as("aria-current")
ariaCurrent: [#page | #step | #location | #date | #time | #\"true" | #\"false"],

This is the same thing using the syntax of bs-platform 9.0.

@optional @as("aria-current")
ariaCurrent: [#page | #step | #location | #date | #time | #"true" | #"false"],

In theory, next release of rescript-react could use bs-platform 9.0, therefore it could make sense to use the newer (and clearer) syntax. But if you think that for now it is better to stick with bs-platform 8.x, I am going to use the old syntax to make the PR 😊

[dodomorandi]

Closing this, because as discussed in #12, all props will be implemented using a ppx, therefore it is not about "how to implement" polyvariants but how the ppx is going to handle the codegen.