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 all commits
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
4 changes: 3 additions & 1 deletion .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
1 change: 1 addition & 0 deletions 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
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
23 changes: 18 additions & 5 deletions src/res_cli.ml
Original file line number Diff line number Diff line change
Expand Up @@ -165,6 +165,7 @@ module ResClflags: sig
val files: string list ref
val interface: bool ref
val report: string ref
val ppx: string ref

val parse: unit -> unit
end = struct
Expand All @@ -178,6 +179,7 @@ end = struct
let origin = ref ""
let interface = ref false
let report = ref "pretty"
let ppx = ref ""

let usage = "Usage:\n rescript <options> <file>\n\n" ^
"Examples:\n" ^
Expand All @@ -192,6 +194,7 @@ end = struct
("-print", Arg.String (fun txt -> print := txt), "Print either binary or ns. Default: ns");
("-width", Arg.Int (fun w -> width := w), "Specify the line length for the printer (formatter)");
("-interface", Arg.Unit (fun () -> interface := true), "Parse as interface");
("-ppx", Arg.String (fun txt -> ppx := txt), "Apply a specific built-in ppx before parsing, none or jsx. Default: none");
(* ("-report", Arg.String (fun txt -> report := txt), "Stylize errors and messages using color and context. Accepts `Pretty` and `Plain`. Default `Plain`") *)
]

Expand All @@ -201,7 +204,7 @@ end
module CliArgProcessor = struct
type backend = Parser: ('diagnostics) Res_driver.parsingEngine -> backend [@@unboxed]

let processFile ~isInterface ~width ~recover ~origin ~target ~report:_ filename =
let processFile ~isInterface ~width ~recover ~origin ~target ~report:_ ~ppx filename =
try
let len = String.length filename in
let processInterface =
Expand Down Expand Up @@ -243,8 +246,12 @@ module CliArgProcessor = struct
else exit 1
end
else
let parsetree = match ppx with
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we keep this a string and match on the string for now? Does this extra conversion give us something in the end?

| "jsx" -> Reactjs_jsx_ppx.rewrite_signature parseResult.parsetree
| _ -> 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
Expand All @@ -258,19 +265,23 @@ module CliArgProcessor = struct
else exit 1
end
else
let parsetree = match ppx with
| "jsx" -> Reactjs_jsx_ppx.rewrite_implementation parseResult.parsetree
| _ -> 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 All @@ -282,6 +293,7 @@ let [@raises exit] () =
~target:!ResClflags.print
~origin:!ResClflags.origin
~report:!ResClflags.report
~ppx:!ResClflags.ppx
""
| files ->
List.iter (fun filename ->
Expand All @@ -292,6 +304,7 @@ let [@raises exit] () =
~target:!ResClflags.print
~origin:!ResClflags.origin
~report:!ResClflags.report
~ppx:!ResClflags.ppx
filename
) files
end
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>
}
101 changes: 101 additions & 0 deletions tests/ppx/react/__snapshots__/render.spec.js.snap
Original file line number Diff line number Diff line change
@@ -0,0 +1,101 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP

exports[`commentAtTop.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\\" = (\\\\\\"Props\\": {\\"msg\\": 'msg}) => make(~msg=\\\\\\"Props\\"[\\"msg\\"])
\\\\\\"CommentAtTop\\"
}
"
`;

exports[`externalWithCustomName.res 1`] = `
"module Foo = {
@bs.obj
external componentProps: (
~a: int,
~b: string,
~key: string=?,
unit,
) => {\\"a\\": int, \\"b\\": string} = \\"\\"
@bs.module(\\"Foo\\")
external component: React.componentLike<
{\\"a\\": int, \\"b\\": string},
React.element,
> = \\"component\\"
}

let t = React.createElement(
Foo.component,
Foo.componentProps(~a=1, ~b={\\"1\\"}, ()),
)
"
`;

exports[`innerModule.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 \`InnerModule.react$Bar\`\\")
ReactDOMRe.createDOMElementVariadic(\\"div\\", [])
}
let make = {
let \\\\\\"InnerModule$Bar\\" = (\\\\\\"Props\\": {\\"a\\": 'a, \\"b\\": 'b}) =>
make(~b=\\\\\\"Props\\"[\\"b\\"], ~a=\\\\\\"Props\\"[\\"a\\"], ())
\\\\\\"InnerModule$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 \`InnerModule.react$Bar$component\`\\")
ReactDOMRe.createDOMElementVariadic(\\"div\\", [])
}
let component = {
let \\\\\\"InnerModule$Bar$component\\" = (\\\\\\"Props\\": {\\"a\\": 'a, \\"b\\": 'b}) =>
component(~b=\\\\\\"Props\\"[\\"b\\"], ~a=\\\\\\"Props\\"[\\"a\\"], ())
\\\\\\"InnerModule$Bar$component\\"
}
}
"
`;

exports[`topLevel.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\\" = (\\\\\\"Props\\": {\\"a\\": 'a, \\"b\\": 'b}) =>
make(~b=\\\\\\"Props\\"[\\"b\\"], ~a=\\\\\\"Props\\"[\\"a\\"], ())
\\\\\\"TopLevel\\"
}
"
`;
6 changes: 6 additions & 0 deletions tests/ppx/react/commentAtTop.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/ppx/react/externalWithCustomName.res
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/ppx/react/innerModule.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 `InnerModule.react$Bar`",
)
<div />
}
@react.component
let component = (~a, ~b, _) => {
Js.log(
"This function should be named `InnerModule.react$Bar$component`",
)
<div />
}
}
1 change: 1 addition & 0 deletions tests/ppx/react/render.spec.js
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
runPrinter(__dirname, "jsx")
5 changes: 5 additions & 0 deletions tests/ppx/react/topLevel.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 />
}
17 changes: 9 additions & 8 deletions tests/runner.js
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,7 @@ function parseNapkinStdinToNapkin(src, isInterface, width = 100) {
.stdout.toString("utf8");
}

function printFile(filename) {
function printFile(filename, ppx) {
let parserSrc;
switch (classifyLang(filename)) {
case "ocaml":
Expand All @@ -165,17 +165,15 @@ function printFile(filename) {
status: 0,
errorOutput: ""
};
break;


case "rescript":
default:
parserSrc = "res";
break;
}

let intf = isInterface(filename);

let args = ["-parse", parserSrc, "-print", "res", "-width", "80"];
let args = ["-parse", parserSrc, "-print", "res", "-width", "80", "-ppx", ppx];

if (intf) {
args.push("-interface");
Expand Down Expand Up @@ -204,15 +202,16 @@ let makeReproducibleFilename = (txt) => {
})
};

global.runPrinter = (dirname) => {
global.runPrinter = (dirname, ppx = "") => {
fs.readdirSync(dirname).forEach((base) => {
let filename = path.join(dirname, base);

if (!fs.lstatSync(filename).isFile() || base === "render.spec.js") {
return;
}

test(base, () => {
let {result, errorOutput, status} = printFile(filename);
let {result, errorOutput, status} = printFile(filename, ppx);
if (status > 0) {
let msg = `Test from file: ${filename} failed with error output:

Expand All @@ -226,7 +225,9 @@ Make sure the test input is syntactically valid.`;
expect(result).toMatchSnapshot();
}

if (process.env.ROUNDTRIP_TEST) {
// Only run roundtrip tests in ppx-free tests.
// Ppxs are only applied in .res syntax, not .re, so resulting ASTs would not match
if (process.env.ROUNDTRIP_TEST && ppx === "") {
let intf = isInterface(filename);
let sexpAst = parseFileToSexp(filename);
let result2 = parseNapkinStdinToNapkin(result, intf, 80);
Expand Down