Deprecate %external extension (#6906)

* deprecate `%external` in favor of `%global` and `%define`

* just deprecate `%external`

Author: cometkim

CHANGELOG.md

@@ -87,6 +87,7 @@
 - Improve unused attribute warning message. https://github.com/rescript-lang/rescript-compiler/pull/6787
 - Remove internal option `use-stdlib` from build schema. https://github.com/rescript-lang/rescript-compiler/pull/6778
 - Fix `Js.Types.JSBigInt` payload to use native `bigint` type. https://github.com/rescript-lang/rescript-compiler/pull/6911
+- Deprecate `%external` extension, which has never been officially introduced. https://github.com/rescript-lang/rescript-compiler/pull/6906
 
 # 11.1.2
 

jscomp/frontend/ast_exp_extension.ml

@@ -62,13 +62,10 @@ let handle_extension e (self : Bs_ast_mapper.mapper)
       (Ast_exp_handle_external.handle_raw ~kind:Raw_re loc payload)
       (Ast_comb.to_js_re_type loc)
   | "external" -> (
+    Location.deprecated loc
+      "%external is deprecated, use %raw or regular FFI syntax instead.";
     match Ast_payload.as_ident payload with
-    | Some {txt = Lident x} ->
-      Ast_exp_handle_external.handle_external loc x
-      (* do we need support [%external gg.xx ]
-
-         {[ Js.Undefined.to_opt (if Js.typeof x == "undefined" then x else Js.Undefined.empty ) ]}
-      *)
+    | Some {txt = Lident x} -> Ast_exp_handle_external.handle_external loc x
     | None | Some _ ->
       Location.raise_errorf ~loc "external expects a single identifier")
   | "time" -> (

jscomp/frontend/ast_exp_handle_external.ml

@@ -24,13 +24,15 @@
 
 open Ast_helper
 
-(*
+(**
    {[
      Js.undefinedToOption 
        (if Js.typeof x = "undefined" then undefined 
         else x  )
 
    ]}
+
+   @deprecated
 *)
 let handle_external loc (x : string) : Parsetree.expression =
   let raw_exp : Ast_exp.t =

jscomp/test/build.ninja

@@ -33,6 +33,12 @@ external createEventNonIEBrowsers: string => Dom.event = "createEvent"
 @send
 external initEventNonIEBrowsers: (Dom.event, string, bool, bool) => unit = "initEvent"
 
+@val @scope("globalThis")
+external window: option<Dom.window> = "window"
+
+@val @scope("globalThis")
+external history: option<Dom.history> = "history"
+
 let safeMakeEvent = eventName =>
   if Js.typeof(event) == "function" {
     makeEventIE11Compatible(eventName)
@@ -60,9 +66,9 @@ let arrayToList = a => {
 /* actually you know what, not gonna provide search for now. It's a mess.
  We'll let users roll their own solution/data structure for now */
 let path = () =>
-  switch %external(window) {
+  switch window {
   | None => list{}
-  | Some(window: Dom.window) =>
+  | Some(window) =>
     switch window |> location |> pathname {
     | ""
     | "/" => list{}
@@ -78,9 +84,9 @@ let path = () =>
     }
   }
 let hash = () =>
-  switch %external(window) {
+  switch window {
   | None => ""
-  | Some(window: Dom.window) =>
+  | Some(window) =>
     switch window |> location |> hash {
     | ""
     | "#" => ""
@@ -91,9 +97,9 @@ let hash = () =>
     }
   }
 let search = () =>
-  switch %external(window) {
+  switch window {
   | None => ""
-  | Some(window: Dom.window) =>
+  | Some(window) =>
     switch window |> location |> search {
     | ""
     | "?" => ""
@@ -103,18 +109,18 @@ let search = () =>
     }
   }
 let push = path =>
-  switch (%external(history), %external(window)) {
+  switch (history, window) {
   | (None, _)
   | (_, None) => ()
-  | (Some(history: Dom.history), Some(window: Dom.window)) =>
+  | (Some(history), Some(window)) =>
     pushState(history, ~href=path)
     dispatchEvent(window, safeMakeEvent("popstate"))
   }
 let replace = path =>
-  switch (%external(history), %external(window)) {
+  switch (history, window) {
   | (None, _)
   | (_, None) => ()
-  | (Some(history: Dom.history), Some(window: Dom.window)) =>
+  | (Some(history), Some(window)) =>
     replaceState(history, ~href=path)
     dispatchEvent(window, safeMakeEvent("popstate"))
   }
@@ -143,17 +149,17 @@ let url = () => {path: path(), hash: hash(), search: search()}
 /* alias exposed publicly */
 let dangerouslyGetInitialUrl = url
 let watchUrl = callback =>
-  switch %external(window) {
+  switch window {
   | None => () => ()
-  | Some(window: Dom.window) =>
+  | Some(window) =>
     let watcherID = () => callback(url())
     addEventListener(window, "popstate", watcherID)
     watcherID
   }
 let unwatchUrl = watcherID =>
-  switch %external(window) {
+  switch window {
   | None => ()
-  | Some(window: Dom.window) => removeEventListener(window, "popstate", watcherID)
+  | Some(window) => removeEventListener(window, "popstate", watcherID)
   }
 
 let useUrl = (~serverUrl=?, ()) => {