rescript-compiler icon indicating copy to clipboard operation
rescript-compiler copied to clipboard

JSX V4

Open cristianoc opened this issue 3 years ago • 32 comments

This task is to track the various activities related to JSX V4.

  • [x] PPX in the syntax repo https://github.com/rescript-lang/syntax/pull/586
  • [x] New configuration with bsconfig.json https://github.com/rescript-lang/rescript-compiler/pull/5484
  • [x] Add support for jsx mode to rescript-react https://github.com/rescript-lang/rescript-react/pull/49
  • [ ] Explore one more instance, such as rescript react native. https://github.com/rescript-react-native/rescript-react-native
  • [x] Spec for V4 (for writers of bindings to frameworks)
  • [x] Upgrade doc for V4
  • [x] Testing on real projects https://github.com/rescript-lang/rescript-compiler/issues/5493 and https://github.com/green-labs/sinsunhi-frontend-mirror
  • [x] Editor support https://github.com/rescript-lang/rescript-compiler/issues/5406
  • [x] GenType support https://github.com/rescript-lang/rescript-compiler/issues/5582
  • [ ] Communication and community engagement https://forum.rescript-lang.org/t/jsx-v4-next-rescript-react-0-10-x/3431
  • [ ] Look into dynamically loaded components https://github.com/rescript-lang/rescript-compiler/issues/5593

cristianoc avatar Jul 04 '22 03:07 cristianoc

@mattdamon108 this topic has grown to be quite substantial. Adding a top-level issue to track the various activities taking place, to coordinate, and so that interested people can follow along.

cristianoc avatar Jul 04 '22 03:07 cristianoc

@mattdamon108 I'm questioning the defaults. Not sure the default should be "do nothing" as in V3. This is likely only confuse people.

How about making v4 react classic the default?

cristianoc avatar Jul 25 '22 05:07 cristianoc

Is it what you want at the beginning? (backward compatibility) 😄 Switching the V4 as default is an effortless task like a finger snap. One thing came to mind is the new jsx config in bsconfig. Switching the V4 as default without the hand-written configuration by the user, I'm afraid it becomes more confusing to our user. I think it looks mismatch between configuration and compilation. What do you think?

mununki avatar Jul 25 '22 08:07 mununki

I think the vast majority of users will never need to change config. And beginners have 1 less thing to learn. Plus there was a forum post right now about an issue due to not setting the configuration, which this would have prevented.

cristianoc avatar Jul 25 '22 09:07 cristianoc

Also, there's no setting for "do nothing". So this is a weird initialization.

cristianoc avatar Jul 25 '22 09:07 cristianoc

I think there is a setting for "do nothing", not adding "reason": { ... } nor "jsx": { ... }. If user doesn't add any of them, jsx ppx will not be triggered.

mununki avatar Jul 25 '22 09:07 mununki

How about throwing more informative error message, in case user didn't add the jsx configuration, but has written JSX expressions or attribute @react.componet?

mununki avatar Jul 25 '22 09:07 mununki

Before further discussion, let me clear your intention.

  1. no configuration at all -> V4 + classic
  2. configuration -> works as configuration

Is it correct?

mununki avatar Jul 25 '22 09:07 mununki

More messages is OK. Except if it's an error it's a breaking change.

cristianoc avatar Jul 25 '22 09:07 cristianoc

I think there is a setting for "do nothing", not adding "reason": { ... } nor "jsx": { ... }. If user doesn't add any of them, jsx ppx will not be triggered.

You can't do that per-file. It's only a config that exists by omitting global config.

cristianoc avatar Jul 25 '22 09:07 cristianoc

I got your point. Let me write down some user stories to make it clear.

First, "reason": 3 + V3 users -> JSX V4 + classic as default If user has error or want to back to V3, then user changes the bsconfig "jsx": { "version": 3 } If user wants to try V4 + automatic, go for it with bsconfig update, "jsx": {"version": 4, "mode": "automatic"}

~~In this regard, only concern is V4 + classic will be triggered even for users who doesn't have any configuration about jsx. I think it is not a big deal. That is how the ppx works normally, regardless of using an attribute or extension which trigger.~~ Does it make sense?

mununki avatar Jul 25 '22 11:07 mununki

Yes, I think making V4 + classic as default brings the new feature with impact.

mununki avatar Jul 25 '22 11:07 mununki

Oh, we can still make not triggering the jsx ppx if user doesn't have any configuration about jsx at all.

mununki avatar Jul 25 '22 11:07 mununki

If we make V4 + classic as default, then submodules in dependencies will be affected by the same default, right? Is it your intention?

mununki avatar Jul 26 '22 00:07 mununki

It is very clever idea! If we update some react component such as Context, Fragment with @react.component then everything will work accordingly! Same to other submodules.

Anyway, some components in this file https://github.com/rescript-lang/rescript-react/pull/49/files#diff-dc0e4227d38d3757b4c1cc61762295e29c7cb769e3644637d7769ea744c9f4d3 , needs to be updated though.

Plus, I'd like to change this PR, and change my initial suggestion. It is better to update React.res with jsx ppx change compatible with @react.component not making RescriptReact.res. (It causes more confusion I think) and It is better to guide users to install rescript-react per their purpose. If they want to use V3, then install pinned version 0.10.* something like that.

mununki avatar Jul 26 '22 00:07 mununki

@mattdamon108 now that 10.0 is out, time to think about 10.1 and JSX v4.

What is the current state? Are there parts still in progress or is it already time for detailed documentation?

cristianoc avatar Aug 26 '22 02:08 cristianoc

Beside the decision about V4 as default, there is no stuffs left in progress. Can you guide me how to fill the detailed documentation?

mununki avatar Aug 26 '22 02:08 mununki

There's a spec which is mostly for implementers, but occasionally could be used by users. I would make sure that it is up to date and clear enough. Then, the changelog needs to be updated with a quick user guide: what are JSX modes, what they do, and how to set them. It's OK to refer to the spec for details. Especially breaking changes. One that is done, one can invite people on the forum to have a go and report feedback.

cristianoc avatar Aug 26 '22 02:08 cristianoc

One thing that came to my mind is to check if the user tries to use async with jsx definition as below. I'll make a PR if there is something to improve the error msg or etc.

@react.component
let make = async () => body

mununki avatar Aug 26 '22 02:08 mununki

@cristianoc I found one thing left to be done. This PR needs to be done https://github.com/rescript-lang/rescript-react/pull/49 which contains the new jsx transform api bindings. The ppx v4 is using it.

mununki avatar Aug 31 '22 00:08 mununki

@cristianoc I found one thing left to be done. This PR needs to be done rescript-lang/rescript-react#49 which contains the new jsx transform api bindings. The ppx v4 is using it.

That is with automatic mode right? The spec should be a bit more explicit about what automatic does, and what are the possible parameters for "mode".

cristianoc avatar Aug 31 '22 01:08 cristianoc

That is with automatic mode right?

Yes, correct.

mununki avatar Aug 31 '22 03:08 mununki

That is with automatic mode right?

Yes, correct.

One more thing, https://github.com/rescript-lang/rescript-react/blob/8f31f375bc8965dad4a1c498b6ab69227f150069/src/RescriptReact.res#L1

RescriptReact module needs to be introduced with jsxConfig({version:r 4}) even though without automatic mode. Because V4 is not a default, React.res module will be transformed by V3.

mununki avatar Sep 01 '22 05:09 mununki

That is with automatic mode right?

Yes, correct.

One more thing, https://github.com/rescript-lang/rescript-react/blob/8f31f375bc8965dad4a1c498b6ab69227f150069/src/RescriptReact.res#L1

RescriptReact module needs to be introduced with jsxConfig({version:r 4}) even though without automatic mode. Because V4 is not a default, React.res module will be transformed by V3.

Can you explain more? There's a lot of content in that sentence.

cristianoc avatar Sep 01 '22 07:09 cristianoc

I can summarize this PR https://github.com/rescript-lang/rescript-react/pull/49 into three things to achieve.

  1. For V4 + classic There are react components that are not defined with @react.component in the current React.res. It means they are working only for V3. The bsconfig.json in rescript-react has jsx configuration of V3 either. Therefore, this PR has RescriptReact.res which contains react components re-written with @react.component + @@jsxConfig({version: 4})

  2. For V4 + automatic = the new jsx transform APIs added https://github.com/rescript-lang/rescript-react/blob/8f31f375bc8965dad4a1c498b6ab69227f150069/src/React.res#L27:L37 https://github.com/rescript-lang/rescript-react/blob/8f31f375bc8965dad4a1c498b6ab69227f150069/src/ReactDOM.res#L2105:L2115

  3. Type binding to Jsx in the compiler https://github.com/rescript-lang/rescript-react/blob/8f31f375bc8965dad4a1c498b6ab69227f150069/src/React.res#L1

So, I like to mention that 1 and 2 in this PR are outstanding stuff for JSX V4.


This is a little bit off-topic and seems proper to discuss in the PR thread.

In my opinion, we can't make a single version supporting both V3 and V4 without having React.res and RescriptReact.res both. I think it is not a happy situation for users.

How about making a breaking version?

  • v0.10.3 for JSX V3
  • v0.11.0 higher for JSX V4

mununki avatar Sep 01 '22 09:09 mununki

Why create a new version for JSX V3?

I understand if some breaking changes might be unavoidable (or unclear how to achieve) in order to support V4. But why a new version for V3?

cristianoc avatar Sep 01 '22 11:09 cristianoc

Oh, I meant v0.10.3 is the current version of rescript-react. There's no need to make a new version for V3. Sorry for making a confusion.

mununki avatar Sep 01 '22 14:09 mununki

@mattdamon108 so I think next steps are publishing a new beta version of rescript-react, finish up any testing and documentation, then invite people to test V4 in its various configurations. Correct?

cristianoc avatar Sep 02 '22 02:09 cristianoc

@mattdamon108 so I think next steps are publishing a new beta version of rescript-react, finish up any testing and documentation, then invite people to test V4 in its various configurations. Correct?

That would be very nice.

mununki avatar Sep 02 '22 02:09 mununki

If you agree to make the breaking version for JSX V4, then I'll fix the PR https://github.com/rescript-lang/rescript-react/pull/49 accordingly.

mununki avatar Sep 02 '22 02:09 mununki