Skip to content

Commit

Permalink
Fix browser_only inside fn
Browse files Browse the repository at this point in the history
  • Loading branch information
davesnx committed Aug 14, 2023
1 parent 30a2b36 commit a70e0c3
Show file tree
Hide file tree
Showing 7 changed files with 73 additions and 22 deletions.
33 changes: 31 additions & 2 deletions packages/ppx/browser.ml
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,28 @@ let effect_rule =
(Extension.V3.declare "effect" Extension.Context.expression extractor
handler)

let browser_only_value_binding pattern expression =
let loc = pattern.ppat_loc in
match pattern with
| [%pat? ()] -> Builder.value_binding ~loc ~pat:pattern ~expr:[%expr ()]
| _ -> (
match expression.pexp_desc with
| Pexp_fun (_arg_label, _arg_expression, fun_pattern, _expression) ->
let stringified = Ppxlib.Pprintast.string_of_expression expression in
let message = Builder.estring ~loc stringified in
Builder.value_binding ~loc ~pat:pattern
~expr:
[%expr
fun [%p fun_pattern] ->
(raise
(ReactDOM.Impossible_in_ssr [%e message]) [@warning "-27"])]
| _ ->
Builder.value_binding ~loc ~pat:pattern
~expr:
[%expr
[%ocaml.error
"browser only works on expressions or function definitions"]])

let browser_only_on_expressions_rule =
let extractor = Ast_pattern.(single_expr_payload __) in
let handler ~ctxt payload =
Expand All @@ -41,12 +63,19 @@ let browser_only_on_expressions_rule =
let message = Builder.estring ~loc stringified in
[%expr
raise ReactDOM.Impossible_in_ssr [%e message] [@warning "-27"]]
| Pexp_fun (_arg_label, _arg_expression, _pattern, _expression) ->
| Pexp_fun (_arg_label, _arg_expression, fun_pattern, _expression) ->
let stringified = Ppxlib.Pprintast.string_of_expression payload in
let message = Builder.estring ~loc stringified in
[%expr
fun () ->
fun [%p fun_pattern] ->
(raise ReactDOM.Impossible_in_ssr [%e message] [@warning "-27"])]
| Pexp_let (rec_flag, value_bindings, expression) ->
Builder.pexp_let ~loc rec_flag
(List.map
(fun binding ->
browser_only_value_binding binding.pvb_pat binding.pvb_expr)
value_bindings)
expression
| _ ->
[%expr
[%ocaml.error
Expand Down
6 changes: 3 additions & 3 deletions packages/ppx/test_snapshot/dune
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
(executable
(name main)
(name test_standalone)
(libraries pipe_first_ppx double_hash_ppx regex_ppx browser_ppx ppxlib))

(rule
(targets ocaml.preprocessed)
(deps ocaml.ml)
(action
(run ./main.exe --impl %{deps} -o %{targets})))
(run ./test_standalone.exe --impl %{deps} -o %{targets})))

(rule
(targets reason.preprocessed)
Expand All @@ -17,7 +17,7 @@
(with-stdout-to
%{targets}
(run refmt --parse re --print ml %{input}))
(run ./main.exe --impl %{targets} -o %{targets})
(run ./test_standalone.exe --impl %{targets} -o %{targets})
(run refmt --parse ml --print re --in-place %{targets}))))

(rule
Expand Down
3 changes: 0 additions & 3 deletions packages/ppx/test_snapshot/ocaml.expected
Original file line number Diff line number Diff line change
Expand Up @@ -93,9 +93,6 @@ let payload_should_be_a_string =
let _ = [%ocaml.error "effect only accepts a useEffect expression"]
let _ = ((raise ReactDOM.Impossible_in_ssr "Webapi.Dom.getElementById")
[@warning "-27"])
let _ =
fun () -> ((raise ReactDOM.Impossible_in_ssr "fun () -> ()")
[@warning "-27"])
let valueFromEvent = Webapi.Dom.getElementById "foo"
let valueFromEvent evt =
raise
Expand Down
1 change: 0 additions & 1 deletion packages/ppx/test_snapshot/ocaml.ml
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,6 @@ let _ = [%effect None]

(* browser_only *)
let _ = [%browser_only Webapi.Dom.getElementById "foo"]
let _ = [%browser_only fun () -> ()]

let%browser_only valueFromEvent = Webapi.Dom.getElementById "foo"
let%browser_only valueFromEvent evt = Webapi.Dom.getElementById "foo"
Expand Down
24 changes: 17 additions & 7 deletions packages/ppx/test_snapshot/reason.expected
Original file line number Diff line number Diff line change
@@ -1,7 +1,17 @@
[@warning "-27"]
let loadInitialText = () =>
raise(
ReactDOM.Impossible_in_ssr(
"fun () ->\n setHtmlFetchState Loading;\n (((((WcApi.fetchHTML WcConstants.initialText) |. Promise.flatMap)\n (fun html ->\n (onChange (html |. WcStringHelpers.htmlToText)) |. Promise.resolved))\n |. Promise.Js.catch)\n (fun _ -> (setHtmlFetchState Error) |. Promise.resolved))\n |. ignore",
),
);
let make = () => {
let loadInitialText = () =>
[@warning "-27"]
raise(
ReactDOM.Impossible_in_ssr(
"fun () ->\n setHtmlFetchState Loading;\n (((((WcApi.fetchHTML WcConstants.initialText) |. Promise.flatMap)\n (fun html ->\n (onChange (html |. WcStringHelpers.htmlToText)) |. Promise.resolved))\n |. Promise.Js.catch)\n (fun _ -> (setHtmlFetchState Error) |. Promise.resolved))\n |. ignore",
),
);
let loadInitialText = argument1 =>
[@warning "-27"]
raise(
ReactDOM.Impossible_in_ssr(
"fun argument1 ->\n fun argument2 ->\n setHtmlFetchState Loading;\n (((((WcApi.fetchHTML WcConstants.initialText) |. Promise.flatMap)\n (fun html ->\n (onChange (html |. WcStringHelpers.htmlToText)) |.\n Promise.resolved))\n |. Promise.Js.catch)\n (fun _ -> (setHtmlFetchState Error) |. Promise.resolved))\n |. ignore",
),
);
React.createElement("div");
};
28 changes: 22 additions & 6 deletions packages/ppx/test_snapshot/reason.re
Original file line number Diff line number Diff line change
@@ -1,7 +1,23 @@
let%browser_only loadInitialText = () => {
setHtmlFetchState(Loading);
WcApi.fetchHTML(WcConstants.initialText)
->Promise.flatMap(html => onChange(html->WcStringHelpers.htmlToText)->Promise.resolved)
->Promise.Js.catch(_ => setHtmlFetchState(Error)->Promise.resolved)
->ignore;
let make = () => {
let%browser_only loadInitialText = () => {
setHtmlFetchState(Loading);
WcApi.fetchHTML(WcConstants.initialText)
->Promise.flatMap(html =>
onChange(html->WcStringHelpers.htmlToText)->Promise.resolved
)
->Promise.Js.catch(_ => setHtmlFetchState(Error)->Promise.resolved)
->ignore;
};

let%browser_only loadInitialText = (argument1, argument2) => {
setHtmlFetchState(Loading);
WcApi.fetchHTML(WcConstants.initialText)
->Promise.flatMap(html =>
onChange(html->WcStringHelpers.htmlToText)->Promise.resolved
)
->Promise.Js.catch(_ => setHtmlFetchState(Error)->Promise.resolved)
->ignore;
};

React.createElement("div");
};
File renamed without changes.

0 comments on commit a70e0c3

Please sign in to comment.