Skip to content
This repository was archived by the owner on Jun 15, 2023. It is now read-only.

Move React ppx from compiler repo, add tests #124

Merged
merged 9 commits into from
Sep 26, 2020
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions .depend
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
src/reactjs_jsx_ppx.cmx : src/reactjs_jsx_ppx.cmi
src/reactjs_jsx_ppx.cmi :
src/res_ast_conversion.cmx : src/res_ast_conversion.cmi
src/res_ast_conversion.cmi :
src/res_ast_debugger.cmx : src/res_driver.cmx src/res_doc.cmx \
Expand All @@ -6,7 +8,7 @@ src/res_ast_debugger.cmi : src/res_driver.cmi
src/res_character_codes.cmx :
src/res_cli.cmx : src/res_driver_reason_binary.cmx \
src/res_driver_ml_parser.cmx src/res_driver_binary.cmx src/res_driver.cmx \
src/res_ast_debugger.cmx
src/res_ast_debugger.cmx src/reactjs_jsx_ppx.cmx
src/res_comment.cmx : src/res_comment.cmi
src/res_comment.cmi :
src/res_comments_table.cmx : src/res_parsetree_viewer.cmx src/res_doc.cmx \
Expand Down Expand Up @@ -45,7 +47,7 @@ src/res_minibuffer.cmi :
src/res_multi_printer.cmx : src/res_printer.cmx src/res_io.cmx \
src/res_driver_reason_binary.cmx src/res_driver_ml_parser.cmx \
src/res_driver.cmx src/res_diagnostics.cmx src/res_ast_conversion.cmx \
src/res_multi_printer.cmi
src/reactjs_jsx_ppx.cmx src/res_multi_printer.cmi
src/res_multi_printer.cmi :
src/res_outcome_printer.cmx : src/res_token.cmx src/res_doc.cmx \
src/res_outcome_printer.cmi
Expand Down
5 changes: 4 additions & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ depend:
$(OCAMLDEP) -native -I tests -I src src/*.ml src/*.mli tests/*.ml tests/*.mli > .depend

API_FILES = \
src/reactjs_jsx_ppx.cmx\
src/res_io.cmx\
src/res_minibuffer.cmx\
src/res_doc.cmx\
Expand Down Expand Up @@ -49,6 +50,8 @@ lib/rescript.exe: $(CLI_FILES)

build-native: lib/refmt.exe lib/rescript.exe depend

build-ppx: src/reactjs_jsx_ppx.cmx
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why does the ppx need a separate build-ppx rule? If it's a part "API_FILES", is this sufficient?


bootstrap: build-native
ocaml unix.cma ./scripts/bspack.ml -bs-main Res_cli -I src -o ./lib/rescript.ml
./lib/rescript.exe -parse ml -print ns ./lib/Rescript.ml > ./lib/Rescript2.ml
Expand Down Expand Up @@ -94,4 +97,4 @@ clean:
rm -rf lib/rescript.exe
rm -rf lib/test.exe
git clean -dfx src
.PHONY: clean test roundtrip-test termination dce exception reanalyze bootstrap build-native
.PHONY: clean test roundtrip-test termination dce exception reanalyze bootstrap build-native build-ppx
880 changes: 880 additions & 0 deletions src/reactjs_jsx_ppx.ml

Large diffs are not rendered by default.

39 changes: 39 additions & 0 deletions src/reactjs_jsx_ppx.mli
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
(*
This is the module that handles turning Reason JSX' agnostic function call into
a ReasonReact-specific function call. Aka, this is a macro, using OCaml's ppx
facilities; https://whitequark.org/blog/2014/04/16/a-guide-to-extension-
points-in-ocaml/
You wouldn't use this file directly; it's used by ReScript's
bsconfig.json. Specifically, there's a field called `react-jsx` inside the
field `reason`, which enables this ppx through some internal call in bsb
*)

(*
There are two different transforms that can be selected in this file (v2 and v3):
v2:
transform `[@JSX] div(~props1=a, ~props2=b, ~children=[foo, bar], ())` into
`ReactDOMRe.createElement("div", ~props={"props1": 1, "props2": b}, [|foo,
bar|])`.
transform `[@JSX] div(~props1=a, ~props2=b, ~children=foo, ())` into
`ReactDOMRe.createElementVariadic("div", ~props={"props1": 1, "props2": b}, foo)`.
transform the upper-cased case
`[@JSX] Foo.createElement(~key=a, ~ref=b, ~foo=bar, ~children=[], ())` into
`ReasonReact.element(~key=a, ~ref=b, Foo.make(~foo=bar, [||]))`
transform `[@JSX] [foo]` into
`ReactDOMRe.createElement(ReasonReact.fragment, [|foo|])`
v3:
transform `[@JSX] div(~props1=a, ~props2=b, ~children=[foo, bar], ())` into
`ReactDOMRe.createDOMElementVariadic("div", ReactDOMRe.domProps(~props1=1, ~props2=b), [|foo, bar|])`.
transform the upper-cased case
`[@JSX] Foo.createElement(~key=a, ~ref=b, ~foo=bar, ~children=[], ())` into
`React.createElement(Foo.make, Foo.makeProps(~key=a, ~ref=b, ~foo=bar, ()))`
transform the upper-cased case
`[@JSX] Foo.createElement(~foo=bar, ~children=[foo, bar], ())` into
`React.createElementVariadic(Foo.make, Foo.makeProps(~foo=bar, ~children=React.null, ()), [|foo, bar|])`
transform `[@JSX] [foo]` into
`ReactDOMRe.createElement(ReasonReact.fragment, [|foo|])`
*)

val rewrite_implementation : Parsetree.structure -> Parsetree.structure

val rewrite_signature : Parsetree.signature -> Parsetree.signature
31 changes: 21 additions & 10 deletions src/res_cli.ml
Original file line number Diff line number Diff line change
Expand Up @@ -207,11 +207,12 @@ module CliArgProcessor = struct
let processInterface =
isInterface || len > 0 && (String.get [@doesNotRaise]) filename (len - 1) = 'i'
in
let parsingEngine =
let parsingEngine, react =
match origin with
| "reasonBinary" -> Parser Res_driver_reason_binary.parsingEngine
| "ml" | "ocaml" -> Parser Res_driver_ml_parser.parsingEngine
| _ -> Parser Res_driver.parsingEngine
| "reasonBinary" -> Parser Res_driver_reason_binary.parsingEngine, false
| "ml" | "ocaml" -> Parser Res_driver_ml_parser.parsingEngine, false
| "reactJsx" -> Parser Res_driver.parsingEngine, true
| _ -> Parser Res_driver.parsingEngine, false
in
let printEngine =
match target with
Expand All @@ -237,40 +238,50 @@ module CliArgProcessor = struct
~source:parseResult.source
~filename:parseResult.filename
parseResult.diagnostics;
if recover || not parseResult.invalid then
if react then
exit 1
else if recover then
printEngine.printInterface
~width ~filename ~comments:parseResult.comments parseResult.parsetree
else ()
end
else
let parsetree =
if react then Reactjs_jsx_ppx.rewrite_signature parseResult.parsetree else parseResult.parsetree
in
printEngine.printInterface
~width ~filename ~comments:parseResult.comments parseResult.parsetree
~width ~filename ~comments:parseResult.comments parsetree
else
let parseResult = backend.parseImplementation ~forPrinter ~filename in
if parseResult.invalid then begin
backend.stringOfDiagnostics
~source:parseResult.source
~filename:parseResult.filename
parseResult.diagnostics;
if recover || not parseResult.invalid then
if react then
exit 1
else if recover then
printEngine.printImplementation
~width ~filename ~comments:parseResult.comments parseResult.parsetree
else ()
end
else
let parsetree =
if react then Reactjs_jsx_ppx.rewrite_implementation parseResult.parsetree else parseResult.parsetree
in
printEngine.printImplementation
~width ~filename ~comments:parseResult.comments parseResult.parsetree
~width ~filename ~comments:parseResult.comments parsetree
with
| Failure txt ->
prerr_string txt;
prerr_newline();
exit 1
| _ -> exit 1
[@@raises exit]
[@@raises Invalid_argument, exit]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What raises Invalid_argument?

Copy link
Contributor Author

@jchavarri jchavarri Sep 22, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The existing react ppx uses that exception to note cases with unexpected situations. I can count at least 19 different places, e.g.

raise
(Invalid_argument
"Key cannot be accessed inside of a component. Don't worry - you can always key a component from its \
parent!")

end


let [@raises exit] () =
let [@raises Invalid_argument, exit] () =
if not !Sys.interactive then begin
ResClflags.parse ();
match !ResClflags.files with
Expand Down
37 changes: 36 additions & 1 deletion src/res_multi_printer.ml
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,40 @@ let printMl ~isInterface ~filename =
~comments:parseResult.comments
parseResult.parsetree

(* print files using React JSX ppx to res syntax *)
let printJsx ~isInterface ~filename =
if isInterface then
let parseResult =
Res_driver.parsingEngine.parseInterface ~forPrinter:true ~filename
in
if parseResult.invalid then
begin
Res_diagnostics.printReport parseResult.diagnostics parseResult.source;
exit 1
end
else
let jsxPass = Reactjs_jsx_ppx.rewrite_signature parseResult.parsetree in
Res_printer.printInterface
~width:defaultPrintWidth
~comments:parseResult.comments
jsxPass
else
let parseResult =
Res_driver.parsingEngine.parseImplementation ~forPrinter:true ~filename
in
if parseResult.invalid then
begin
Res_diagnostics.printReport parseResult.diagnostics parseResult.source;
exit 1
end
else
let jsxPass = Reactjs_jsx_ppx.rewrite_implementation parseResult.parsetree in
Res_printer.printImplementation
~width:defaultPrintWidth
~comments:parseResult.comments
jsxPass
[@@raises Invalid_argument, Failure, exit]

(* How does printing Reason to Res work?
* -> open a tempfile
* -> write the source code found in "filename" into the tempfile
Expand Down Expand Up @@ -122,7 +156,8 @@ let print language ~input =
len > 0 && String.unsafe_get input (len - 1) = 'i'
in
match language with
| `jsx -> printJsx ~isInterface ~filename:input
| `res -> printRes ~isInterface ~filename:input
| `ml -> printMl ~isInterface ~filename:input
| `refmt path -> printReason ~refmtPath:path ~isInterface ~filename:input
[@@raises Sys_error, exit]
[@@raises Invalid_argument, Failure, Sys_error, exit]
2 changes: 1 addition & 1 deletion src/res_multi_printer.mli
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
(* Interface to print source code from different languages to res.
* Takes a filename called "input" and returns the corresponding formatted res syntax *)
val print: [`ml | `res | `refmt of string (* path to refmt *)] -> input: string -> string
val print: [`jsx | `ml | `res | `refmt of string (* path to refmt *)] -> input: string -> string
6 changes: 6 additions & 0 deletions tests/api/resReactJsx.res
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
// test React JSX file

@react.component
let make = (~msg) => {
<div> {msg->React.string} </div>
}
88 changes: 88 additions & 0 deletions tests/printer/reactJsx/__snapshots__/render.spec.js.snap
Original file line number Diff line number Diff line change
@@ -0,0 +1,88 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if we should make a separate folder for ppx tests: tests/ppx/...? What do you think?


exports[`commentAtTop.react.res 1`] = `
"@bs.obj
external makeProps: (
~msg: 'msg,
~key: string=?,
unit,
) => {
\\"msg\\": 'msg,
// test React JSX file
} = \\"\\"

let make =
(@warning(\\"-16\\") ~msg) => {
ReactDOMRe.createDOMElementVariadic(\\"div\\", [{msg->React.string}])
}
let make = {
let \\\\\\"CommentAtTop.react\\" = (\\\\\\"Props\\": {\\"msg\\": 'msg}) =>
make(~msg=\\\\\\"Props\\"[\\"msg\\"])
\\\\\\"CommentAtTop.react\\"
}
"
`;

exports[`externalWithCustomName.react.res.fixme 1`] = `""`;

exports[`innerModule.react.res 1`] = `
"module Bar = {
@bs.obj
external makeProps: (
~a: 'a,
~b: 'b,
~key: string=?,
unit,
) => {\\"a\\": 'a, \\"b\\": 'b} = \\"\\"
let make =
(@warning(\\"-16\\") ~a, @warning(\\"-16\\") ~b, _) => {
Js.log(\\"This function should be named \`Test$Bar\`\\")
ReactDOMRe.createDOMElementVariadic(\\"div\\", [])
}
let make = {
let \\\\\\"InnerModule.react$Bar\\" = (\\\\\\"Props\\": {\\"a\\": 'a, \\"b\\": 'b}) =>
make(~b=\\\\\\"Props\\"[\\"b\\"], ~a=\\\\\\"Props\\"[\\"a\\"], ())
\\\\\\"InnerModule.react$Bar\\"
}
@bs.obj
external componentProps: (
~a: 'a,
~b: 'b,
~key: string=?,
unit,
) => {\\"a\\": 'a, \\"b\\": 'b} = \\"\\"

let component =
(@warning(\\"-16\\") ~a, @warning(\\"-16\\") ~b, _) => {
Js.log(\\"This function should be named \`Test$Bar$component\`\\")
ReactDOMRe.createDOMElementVariadic(\\"div\\", [])
}
let component = {
let \\\\\\"InnerModule.react$Bar$component\\" = (\\\\\\"Props\\": {\\"a\\": 'a, \\"b\\": 'b}) =>
component(~b=\\\\\\"Props\\"[\\"b\\"], ~a=\\\\\\"Props\\"[\\"a\\"], ())
\\\\\\"InnerModule.react$Bar$component\\"
}
}
"
`;

exports[`topLevel.react.res 1`] = `
"@bs.obj
external makeProps: (
~a: 'a,
~b: 'b,
~key: string=?,
unit,
) => {\\"a\\": 'a, \\"b\\": 'b} = \\"\\"
let make =
(@warning(\\"-16\\") ~a, @warning(\\"-16\\") ~b, _) => {
Js.log(\\"This function should be named 'TopLevel.react'\\")
ReactDOMRe.createDOMElementVariadic(\\"div\\", [])
}
let make = {
let \\\\\\"TopLevel.react\\" = (\\\\\\"Props\\": {\\"a\\": 'a, \\"b\\": 'b}) =>
make(~b=\\\\\\"Props\\"[\\"b\\"], ~a=\\\\\\"Props\\"[\\"a\\"], ())
\\\\\\"TopLevel.react\\"
}
"
`;
6 changes: 6 additions & 0 deletions tests/printer/reactJsx/commentAtTop.react.res
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
// test React JSX file

@react.component
let make = (~msg) => {
<div> {msg->React.string} </div>
}
6 changes: 6 additions & 0 deletions tests/printer/reactJsx/externalWithCustomName.react.res.fixme
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
module Foo = {
@react.component @bs.module("Foo")
external component: (~a: int, ~b: string, _) => React.element = "component"
}

let t = <Foo.component a=1 b={"1"} />
16 changes: 16 additions & 0 deletions tests/printer/reactJsx/innerModule.react.res
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
module Bar = {
@react.component
let make = (~a, ~b, _) => {
Js.log(
"This function should be named `Test$Bar`",
)
<div />
}
@react.component
let component = (~a, ~b, _) => {
Js.log(
"This function should be named `Test$Bar$component`",
)
<div />
}
}
1 change: 1 addition & 0 deletions tests/printer/reactJsx/render.spec.js
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
runPrinter(__dirname, false, false)
5 changes: 5 additions & 0 deletions tests/printer/reactJsx/topLevel.react.res
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
@react.component
let make = (~a, ~b, _) => {
Js.log("This function should be named 'TopLevel.react'")
<div />
}
25 changes: 25 additions & 0 deletions tests/res_test.ml
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,31 @@ let y: float
|}
)

(* test printing of React jsx file *)
let () =
let filename = "./tests/api/resReactJsx.res" in
let prettySource = Res_multi_printer.print `jsx ~input:filename in
assert (
prettySource = {|@bs.obj
external makeProps: (
~msg: 'msg,
~key: string=?,
unit,
) => {
"msg": 'msg,
// test React JSX file
} = ""

let make =
(@warning("-16") ~msg) => {
ReactDOMRe.createDOMElementVariadic("div", [{msg->React.string}])
}
let make = {
let \"ResReactJsx" = (\"Props": {"msg": 'msg}) => make(~msg=\"Props"["msg"])
\"ResReactJsx"
}
|})

let () = print_endline "✅ multi printer api tests"

module OutcomePrinterTests = struct
Expand Down
Loading