Merge pull request #6 from melange-community/undo-instanceof-pr

Revert "Use `instanceof` to safely downcast (#182)"
melange-community · Aug 23, 2023 · 3fa3259 · 3fa3259
2 parents 17a6998 + 0e13ed6
commit 3fa3259
Show file tree

Hide file tree

Showing 5 changed files with 50 additions and 53 deletions.
diff --git a/src/Webapi/Canvas/Webapi__Canvas__Canvas2d.re b/src/Webapi/Canvas/Webapi__Canvas__Canvas2d.re
@@ -112,22 +112,25 @@ let setFillStyle = (type a, ctx: t, _: style(a), v: a) =>
   setFillStyle(ctx, v);
 
 let reifyStyle = (type a, x: 'a) : (style(a), a) => {
-  let isCanvasGradient: 'a => bool = [%raw {|
-    function(x) { return x instanceof CanvasGradient; }
-  |}];
-
-  let isCanvasPattern: 'a => bool = [%raw {|
-    function(x) { return x instanceof CanvasPattern; }
-  |}];
+  module Internal = {
+    type constructor;
+    [@bs.val] external canvasGradient : constructor = "CanvasGradient"; /* internal */
+    [@bs.val] external canvasPattern : constructor = "CanvasPattern"; /* internal */
+    let instanceOf: ('a, constructor) => bool = [%bs.raw {|function(x,y) {return +(x instanceof y)}|}]; /* internal */
+  };
 
   (
-    if (Js.typeof(x) == "string") Obj.magic(String)
-    else if (isCanvasGradient(x)) Obj.magic(Gradient)
-    else if (isCanvasPattern(x)) Obj.magic(Pattern)
-    else invalid_arg(
-      "Unknown canvas style kind. Known values are: String, CanvasGradient, CanvasPattern"),
+    if (Js.typeof(x) == "string") {
+      Obj.magic(String)
+    } else if (Internal.instanceOf(x, Internal.canvasGradient)) {
+      Obj.magic(Gradient)
+    } else if (Internal.instanceOf(x, Internal.canvasPattern)) {
+      Obj.magic(Pattern)
+    } else {
+      raise(Invalid_argument("Unknown canvas style kind. Known values are: String, CanvasGradient, CanvasPattern"))
+    },
     Obj.magic(x)
-  );
+  )
 };
 
 [@bs.get] external fillStyle : t => 'a = "fillStyle";

diff --git a/src/Webapi/Dom/Webapi__Dom__Document.re b/src/Webapi/Dom/Webapi__Dom__Document.re
@@ -1,21 +1,17 @@
 module Impl = (T: {type t;}) => {
   external asDocument : T.t => Dom.document = "%identity";
 
-  let asHtmlDocument: T.t => option(Dom.htmlDocument) = [%raw {|
-    function(document) {
-      var defaultView = document.defaultView;
-
-      if (defaultView != null) {
-        var HTMLDocument = defaultView.HTMLDocument;
-
-        if (HTMLDocument != null && document instanceof HTMLDocument) {
-          return document;
-        }
-      }
+  let asHtmlDocument: T.t => Js.null(Dom.htmlDocument) = [%raw
+    {|
+    function (document) {
+      return document.doctype.name === "html" ?  document : null;
     }
-  |}];
+  |}
+  ];
+  [@deprecated "Will fail if no doctype is defined, consider using unsafeAsHtmlDocument instead"]
+  let asHtmlDocument: T.t => option(Dom.htmlDocument) =
+    (self) => Js.Null.toOption(asHtmlDocument(self));
 
-  /** Unsafe cast, use [ashtmlDocument] instead */
   external unsafeAsHtmlDocument : T.t => Dom.htmlDocument = "%identity";
 
   let ofNode = (node: Dom.node) : option(T.t) =>

diff --git a/src/Webapi/Dom/Webapi__Dom__Element.re b/src/Webapi/Dom/Webapi__Dom__Element.re
@@ -5,25 +5,18 @@ let ofNode = (node: Dom.node) : option('a) =>
     None;
 
 module Impl = (T: {type t;}) => {
-  let asHtmlElement: T.t => option(Dom.htmlElement) = [%raw {|
-    function(element) {
-      var ownerDocument = element.ownerDocument;
-
-      if (ownerDocument != null) {
-        var defaultView = ownerDocument.defaultView;
-
-        if (defaultView != null) {
-          var HTMLElement = defaultView.HTMLElement;
-
-          if (HTMLElement != null && element instanceof HTMLElement) {
-            return element;
-          }
-        }
-      }
+  let asHtmlElement: T.t => Js.null(Dom.htmlElement) = [%raw
+    {|
+    function (element) {
+      // BEWARE: Assumes "contentEditable" uniquely identifies an HTMLELement
+      return element.contentEditable !== undefined ?  element : null;
     }
-  |}];
+  |}
+  ];
+  [@deprecated "asHtmlElement uses a weak heuristic, consider using unsafeAsHtmlElement instead"]
+  let asHtmlElement: T.t => option(Dom.htmlElement) =
+    (self) => Js.Null.toOption(asHtmlElement(self));
 
-  /** Unsafe cast, use [asHtmlElement] instead */
   external unsafeAsHtmlElement : T.t => Dom.htmlElement = "%identity";
 
   let ofNode: Dom.node => option(T.t) = ofNode;

diff --git a/src/Webapi/Dom/Webapi__Dom__HtmlElement.re b/src/Webapi/Dom/Webapi__Dom__HtmlElement.re
@@ -1,7 +1,16 @@
 module Impl = (T: {type t;}) => {
   type t_htmlElement = T.t;
 
-  let ofElement = Webapi__Dom__Element.asHtmlElement;
+  let ofElement: Dom.element => Js.null(t_htmlElement) = [%bs.raw
+    {|
+    function (element) {
+      // BEWARE: Assumes "contentEditable" uniquely identifies an HTMLELement
+      return element.contentEditable !== undefined ?  element : null;
+    }
+  |}
+  ];
+  [@deprecated "Consider using Element.asHtmlElement or Element.unsafeAsHtmlElement instead"]
+  let ofElement: Dom.element => option(t_htmlElement) = (self) => Js.Null.toOption(ofElement(self));
 
   [@bs.get] external accessKey : t_htmlElement => string = "accessKey";
   [@bs.set] external setAccessKey : (t_htmlElement, string) => unit = "accessKey";

diff --git a/tests/Webapi/Dom/Webapi__Dom__DomStringMap__test.re b/tests/Webapi/Dom/Webapi__Dom__DomStringMap__test.re
@@ -4,13 +4,9 @@ open Webapi.Dom.DomStringMap;
 let dataset =
   document
   |> Document.createElement("div")
-  |> Element.asHtmlElement
-  |> Belt.Option.map(_, HtmlElement.dataset);
+  |> Element.unsafeAsHtmlElement
+  |> HtmlElement.dataset;
 
-let () = switch (dataset) {
-  | Some(dataset) =>
-    set("fooKey", "barValue", dataset);
-    dataset |> get("fooKey") |> ignore;
-    unsafeDeleteKey("fooKey", dataset);
-  | None => ()
-};
+let () = set("fooKey", "barValue", dataset);
+let _ = get("fooKey", dataset);
+let () = unsafeDeleteKey("fooKey", dataset);