Wire to JSX PPX V4 and introduce Jsx* modules #5484

mununki · 2022-06-27T18:42:09Z

Wire to JSX PPX V4

Companion PR in the syntax repo: rescript-lang/syntax#614

Add the new configuration in bsconfig.json

"react": {
  "jsx": 4,
  "mode": "react",
  "runtime": "automatic"
}

Add cli options to wire to the JSX PPX

Introduce `Jsx*` modules

~~WIP: Add Js.React for the new jsx transform binding~~ Add Jsx module for the primitive types and bindings
- Add JsxDOM with type domProps
- Add JsxDOMStyle
- Add JsxEvent

docs/docson/build-schema.json

jscomp/bsb/bsb_config_parse.ml

cristianoc · 2022-06-28T02:10:06Z

jscomp/bsb/bsb_config_parse.ml

+               match flo with
+               | "3" -> default := Some Jsx_v3
+               | "4" -> default := Some Jsx_v4
+               | _ -> Bsb_exception.errorf ~loc "Unsupported react-jsx %s" flo)


"Unsupported jsx version"

Fixed 81029b3

jscomp/bsb/bsb_config_parse.ml

jscomp/others/js.ml

ryyppy · 2022-06-29T16:04:51Z

Kinda related: is this the time where we could think about moving out of the react config namespace? I never liked that category name, since it didn't make sense from a config hierarchy perspective.

My ideal world would show something like

"jsx": {
  "version": 4,
  "jsxModule": "Jsx",
  "mode": "preserve"
  // etc
}

The original react namespace should stay for backwards compat of course, so this would require some errors when a user defined both configs.

mununki · 2022-06-29T18:10:17Z

Kinda related: is this the time where we could think about moving out of the react config namespace? I never liked that category name, since it didn't make sense from a config hierarchy perspective.

My ideal world would show something like
"jsx": {
  "version": 4,
  "jsxModule": "Jsx",
  "mode": "preserve"
  // etc
}
The original react namespace should stay for backwards compat of course, so this would require some errors when a user defined both configs.

It looks good to me. What are the values available for jsxModule anyway?

mununki · 2022-06-29T18:26:16Z

I’m going to remove the Js.React module which was for the test flight of ppx v4.

ryyppy · 2022-06-30T06:10:11Z

It looks good to me. What are the values available for jsxModule anyway?

If technically possible, it should be any value.

As an example: jsxModule: "React" would require me to have a global React module available in my build.
Or it could be something completely different, like "MyCoolJsx".

cometkim · 2022-06-30T12:36:43Z

Many JS frameworks other than React use JSX today.

Preact uses jsx pragma with "h" func: https://preactjs.com/guide/v8/getting-started/#global-pragma

Vue also uses "h": https://vuejs.org/guide/extras/render-function.html#JSX

Emotion uses "jsx": https://emotion.sh/docs/css-prop#jsx-pragma

Theme-UI guides user to customize "jsxImportSource" option: https://theme-ui.com/guides/jsx-pragma/

Btw in case of Emotion/Theme-UI, it allows additional dom props in the file, which is challenging in ReScript I guess

mununki · 2022-07-03T09:16:46Z

Kinda related: is this the time where we could think about moving out of the react config namespace? I never liked that category name, since it didn't make sense from a config hierarchy perspective.

My ideal world would show something like
"jsx": {
  "version": 4,
  "jsxModule": "Jsx",
  "mode": "preserve"
  // etc
}
The original react namespace should stay for backwards compat of course, so this would require some errors when a user defined both configs.

81029b3 I've changed the build schema for bsconfig as suggested.

"jsx": {
  "version": 4,
  "module": "react",
  "mode": "automatic", 
}

Actually, "module" has to be "react" only for now. "mode" is enum type which has classic, automatic. classic is for backward compatibility, automatic is for the new jsx transform for react. This configuration seems quite flexible to support other JSX libraries or frameworks in the future.

cristianoc · 2022-07-03T13:33:25Z

How does one turn on V4? I guess using the new "jsx" config?
That seems good to me.

cristianoc · 2022-07-03T13:34:52Z

Why the name "module". Not a big deal but maybe there's a better name?
Framework or something familiar to the user.

mununki · 2022-07-03T15:02:50Z

How does one turn on V4? I guess using the new "jsx" config? That seems good to me.

Yes, "jsx" turns the V4 on.

mununki · 2022-07-03T16:03:23Z

Why the name "module". Not a big deal but maybe there's a better name? Framework or something familiar to the user.

That was from the "jsxModule" which was in Patrick's comment. It means the value of jsxModule is activated through the build system. Therefore it makes sense to me in a way that the "module" is activated. Sure, we can consider another better name for it.

jscomp/bsb/bsb_config_parse.ml

jscomp/bsb/bsb_merlin_gen.ml

cristianoc · 2022-07-04T02:29:01Z

jscomp/bsb/bsb_ninja_rule.ml

-    (match (has_reason_react_jsx, reason_react_jsx) with
-    | false, _ | _, None -> ()
-    | _, Some Jsx_v3 -> Ext_buffer.add_string buf " -bs-jsx 3");
+    (match (has_reason_react_jsx, reason_react_jsx, jsx_version) with


The first value is never used.

Oops, I'll remove it

jscomp/bsb/bsb_ninja_rule.ml

cristianoc · 2022-07-04T02:34:32Z

jscomp/frontend/ppx_entry.ml

@@ -30,6 +30,7 @@ let rewrite_signature (ast : Parsetree.signature) : Parsetree.signature =
  let ast =
    match !Js_config.jsx_version with
    | 3 -> Reactjs_jsx_ppx_v3.rewrite_signature ast
+    | 4 -> Reactjs_jsx_ppx_v4.rewrite_signature !Js_config.jsx_mode ast


Style: perhaps use a labeled argument ~jsx_mode here as first argument to Reactjs_jsx_ppx_v4.rewrite_signature.

jscomp/others/jsx.ml

jscomp/others/jsxDOM.ml

cristianoc · 2022-07-19T01:02:21Z

Can you rebase this and point it to the latest syntax PR?
Difficult to test the syntax PR otherwise.

mununki · 2022-07-19T01:40:44Z

Rebased to master

cristianoc · 2022-07-19T01:50:53Z

Looks like it's not pointing to the correct syntax commit. Or some code needs changing.

cristianoc · 2022-07-19T01:51:34Z

File "jscomp/frontend/ppx_entry.ml", line 38, characters 8-41:
Error: Unbound module Reactjs_jsx_ppx
Hint: Did you mean Reactjs_jsx_ppx_v3 or Reactjs_jsx_ppx_v4?

cristianoc · 2022-07-19T02:00:00Z

I guess for adding the syntax repo commit here, it's necessary that the syntax PR is done on a branch of the syntax repo, not on a fork. You should be able to do that now.

mununki · 2022-07-19T02:19:24Z

I guess for adding the syntax repo commit here, it's necessary that the syntax PR is done on a branch of the syntax repo, not on a fork. You should be able to do that now.

Got your invitation, I'll make a branch for V4 in the syntax repo, not from my forked repo, then point to the commit of it.

mununki · 2022-07-19T02:27:37Z

I guess for adding the syntax repo commit here, it's necessary that the syntax PR is done on a branch of the syntax repo, not on a fork. You should be able to do that now.

I think I've done it.

cristianoc · 2022-07-19T04:08:17Z

Done a bit of undocumented black magic to fix CI. Let's see if it works.

cristianoc · 2022-07-19T04:20:51Z

OK CI all green now.

cristianoc · 2022-07-21T01:46:30Z

Would you merge this on top of master, which already contains the syntax update?

cristianoc · 2022-07-21T02:49:10Z

In terms of functionality, can this be merged?
If yes, I could do one pass of deeper review.

mununki · 2022-07-21T06:29:30Z

In terms of functionality, can this be merged? If yes, I could do one pass of deeper review.

Yes, I think it is ready to be merged. Let me rebase on top of master.

mununki · 2022-07-21T07:04:54Z

I've pushed this PR branch to upstream mistakenly, you can ignore or delete it. I was confused where this PR branch belongs to. 😓 I've pushed to my forked repo(origin), and this PR seems updated okay.

mununki changed the title ~~JSX PPX V4~~ Wire to JSX PPX V4 Jun 27, 2022

cristianoc reviewed Jun 28, 2022

View reviewed changes

docs/docson/build-schema.json Outdated Show resolved Hide resolved

cristianoc reviewed Jun 28, 2022

View reviewed changes

docs/docson/build-schema.json Outdated Show resolved Hide resolved

cristianoc reviewed Jun 28, 2022

View reviewed changes

jscomp/bsb/bsb_config_parse.ml Outdated Show resolved Hide resolved

cristianoc reviewed Jun 28, 2022

View reviewed changes

jscomp/bsb/bsb_config_parse.ml Outdated Show resolved Hide resolved

mununki mentioned this pull request Jun 29, 2022

JSX PPX V4 part2 rescript-lang/syntax#586

Closed

13 tasks

ryyppy reviewed Jun 29, 2022

View reviewed changes

jscomp/others/js.ml Outdated Show resolved Hide resolved

mununki closed this Jun 29, 2022

mununki reopened this Jun 29, 2022

mununki mentioned this pull request Jul 2, 2022

Update new jsx transform apis, bind to Jsx in compiler, for JSX V4. rescript-lang/rescript-react#49

Merged

mununki force-pushed the jsx-ppx-v4 branch 2 times, most recently from 8c97546 to dec3057 Compare July 3, 2022 07:27