From 0e13ed6a6fae57e9c98f2286b29f9b9d9a234686 Mon Sep 17 00:00:00 2001 From: Javier Chavarri Date: Wed, 23 Aug 2023 13:03:33 +0000 Subject: [PATCH] Revert "Use `instanceof` to safely downcast (#182)" This reverts commit 17b2822393a2e06429e835d5554d592a7273780d. --- src/Webapi/Canvas/Webapi__Canvas__Canvas2d.re | 29 ++++++++++--------- src/Webapi/Dom/Webapi__Dom__Document.re | 22 ++++++-------- src/Webapi/Dom/Webapi__Dom__Element.re | 27 +++++++---------- src/Webapi/Dom/Webapi__Dom__HtmlElement.re | 11 ++++++- .../Dom/Webapi__Dom__DomStringMap__test.re | 14 ++++----- 5 files changed, 50 insertions(+), 53 deletions(-) diff --git a/src/Webapi/Canvas/Webapi__Canvas__Canvas2d.re b/src/Webapi/Canvas/Webapi__Canvas__Canvas2d.re index a62de54..14e7db2 100644 --- 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 index ccb2803..aad7e46 100644 --- 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 index ab4759c..a8fc919 100644 --- 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 index 86ce069..9d2fb44 100644 --- 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 index 6719f65..f127dd5 100644 --- 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);