From 8b900ef34efa04c8e73c90682b8d4dc5731f2c05 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Javier=20Ch=C3=A1varri?= <javier.chavarri@gmail.com>
Date: Wed, 19 Jun 2024 15:54:20 +0200
Subject: [PATCH] browser_only: don't convert to runtime errors on identifiers
 or function application (#138)

---
 packages/browser-ppx/ppx.ml             | 20 +++-----------------
 packages/browser-ppx/tests/pexp_apply.t | 14 ++++++++++----
 packages/browser-ppx/tests/pexp_ident.t |  8 +++++---
 3 files changed, 18 insertions(+), 24 deletions(-)

diff --git a/packages/browser-ppx/ppx.ml b/packages/browser-ppx/ppx.ml
index 0171df6b9..687d0da65 100644
--- a/packages/browser-ppx/ppx.ml
+++ b/packages/browser-ppx/ppx.ml
@@ -92,9 +92,9 @@ module Browser_only = struct
   let error_only_works_on ~loc =
     [%expr
       [%ocaml.error
-        "[browser_ppx] browser_only works on function definitions or values. \
-         If there's another case where it can be helpful, feel free to open an \
-         issue in https://github.com/ml-in-barcelona/server-reason-react."]]
+        "[browser_ppx] browser_only works on function definitions. For other \
+         cases, use switch%platform or feel free to open an issue in \
+         https://github.com/ml-in-barcelona/server-reason-react."]]
 
   let remove_alert_browser_only ~loc =
     Builder.attribute ~loc ~name:{ txt = "alert"; loc }
@@ -135,14 +135,6 @@ module Browser_only = struct
             in
             let vb = Builder.value_binding ~loc ~pat:pattern ~expr in
             { vb with pvb_attributes = [ remove_alert_browser_only ~loc ] }
-        | Pexp_ident { txt = longident; loc } ->
-            let stringified = Ppxlib.Longident.name longident in
-            let message = Builder.estring ~loc stringified in
-            let vb =
-              Builder.value_binding ~loc ~pat:pattern
-                ~expr:[%expr Runtime.fail_impossible_action_in_ssr [%e message]]
-            in
-            { vb with pvb_attributes = [ remove_alert_browser_only ~loc ] }
         | _ ->
             Builder.value_binding ~loc ~pat:pattern
               ~expr:(error_only_works_on ~loc))
@@ -155,12 +147,6 @@ module Browser_only = struct
     | Js -> payload
     | Native -> (
         match payload.pexp_desc with
-        | Pexp_apply (expression, _) ->
-            let stringified =
-              Ppxlib.Pprintast.string_of_expression expression
-            in
-            let message = Builder.estring ~loc stringified in
-            [%expr Runtime.fail_impossible_action_in_ssr [%e message]]
         | Pexp_constraint
             ( {
                 pexp_desc =
diff --git a/packages/browser-ppx/tests/pexp_apply.t b/packages/browser-ppx/tests/pexp_apply.t
index a0ac06e8d..c74dab1ac 100644
--- a/packages/browser-ppx/tests/pexp_apply.t
+++ b/packages/browser-ppx/tests/pexp_apply.t
@@ -14,15 +14,21 @@ With -js flag everything keeps as it is and browser_only extension disappears
   let make () =
     let pstr_value_binding_2 = Webapi.Dom.getElementById "foo" in
     ()
+
+Without -js flag, the compilation to native errors out indicating that a function must be used
+
   $ ./standalone.exe -impl input.ml | ocamlformat - --enable-outside-detected-project --impl
   let pstr_value_binding =
-    Runtime.fail_impossible_action_in_ssr "Webapi.Dom.getElementById"
+    [%ocaml.error
+      "[browser_ppx] browser_only works on function definitions. For other \
+       cases, use switch%platform or feel free to open an issue in \
+       https://github.com/ml-in-barcelona/server-reason-react."]
   
   let make () =
     let pstr_value_binding_2 =
       [%ocaml.error
-        "[browser_ppx] browser_only works on function definitions or values. If \
-         there's another case where it can be helpful, feel free to open an \
-         issue in https://github.com/ml-in-barcelona/server-reason-react."]
+        "[browser_ppx] browser_only works on function definitions. For other \
+         cases, use switch%platform or feel free to open an issue in \
+         https://github.com/ml-in-barcelona/server-reason-react."]
     in
     ()
diff --git a/packages/browser-ppx/tests/pexp_ident.t b/packages/browser-ppx/tests/pexp_ident.t
index a211f9798..ca689da2a 100644
--- a/packages/browser-ppx/tests/pexp_ident.t
+++ b/packages/browser-ppx/tests/pexp_ident.t
@@ -11,12 +11,14 @@ With -js flag everything keeps as it is and browser_only extension disappears
     let pexp_ident = Webapi__Dom__Element.asHtmlElement in
     ()
 
-Without -js flag, the compilation to native replaces the expression with `Runtime.fail_impossible_action_in_ssr`
+Without -js flag, the compilation to native errors out indicating that a function must be used
 
   $ ./standalone.exe -impl input.ml | ocamlformat - --enable-outside-detected-project --impl
   let make () =
     let pexp_ident =
-      Runtime.fail_impossible_action_in_ssr "Webapi__Dom__Element.asHtmlElement"
-        [@@alert "-browser_only"]
+      [%ocaml.error
+        "[browser_ppx] browser_only works on function definitions. For other \
+         cases, use switch%platform or feel free to open an issue in \
+         https://github.com/ml-in-barcelona/server-reason-react."]
     in
     ()