tojs: misc cleanup

* optimize toJS set
* change defineProperty wrappers to return an enum

If we're going to wrap defineProperty, then let's do it properly.

5 files changed, 49 insertions, 22 deletions

diff --git a/src/config/config.nim b/src/config/config.nim
index 3909e46c..0dbc6877 100644
--- a/src/config/config.nim
+++ b/src/config/config.nim
@@ -826,7 +826,9 @@ proc initCommands*(config: Config): Err[string] =
         var prop = JS_GetPropertyStr(ctx, objIt, cstring(ss))
         if JS_IsUndefined(prop):
           prop = JS_NewObject(ctx)
-          ctx.definePropertyE(objIt, ss, JS_DupValue(ctx, prop))
+          case ctx.definePropertyE(objIt, ss, JS_DupValue(ctx, prop))
+          of dprException: return err(ctx.getExceptionMsg())
+          else: discard
         if JS_IsException(prop):
           return err(ctx.getExceptionMsg())
         JS_FreeValue(ctx, objIt)
@@ -840,7 +842,9 @@ proc initCommands*(config: Config): Err[string] =
     if not JS_IsFunction(ctx, fun):
       JS_FreeValue(ctx, fun)
       return err(k & " is not a function")
-    ctx.definePropertyE(objIt, name, JS_DupValue(ctx, fun))
+    case ctx.definePropertyE(objIt, name, JS_DupValue(ctx, fun))
+    of dprException: return err(ctx.getExceptionMsg())
+    else: discard
     config.cmd.map[k] = fun
     JS_FreeValue(ctx, objIt)
   config.cmd.jsObj = JS_DupValue(ctx, obj)
diff --git a/src/html/dom.nim b/src/html/dom.nim
index b0e98d78..48d6655c 100644
--- a/src/html/dom.nim
+++ b/src/html/dom.nim
@@ -1476,7 +1476,11 @@ proc parentNodeChildrenImpl(ctx: JSContext; parentNode: Node): JSValue =
     childonly = true
   ))
   let this = ctx.toJS(parentNode)
-  ctx.definePropertyCW(this, "children", JS_DupValue(ctx, children))
+  case ctx.definePropertyCW(this, "children", JS_DupValue(ctx, children))
+  of dprException:
+    JS_FreeValue(ctx, this)
+    return JS_EXCEPTION
+  else: discard
   JS_FreeValue(ctx, this)
   return children
 
@@ -1497,7 +1501,11 @@ func childNodes(ctx: JSContext; node: Node): JSValue {.jsfget.} =
     childonly = true
   ))
   let this = ctx.toJS(node)
-  ctx.definePropertyCW(this, "childNodes", JS_DupValue(ctx, childNodes))
+  case ctx.definePropertyCW(this, "childNodes", JS_DupValue(ctx, childNodes))
+  of dprException:
+    JS_FreeValue(ctx, this)
+    return JS_EXCEPTION
+  else: discard
   JS_FreeValue(ctx, this)
   return childNodes
 
@@ -1849,10 +1857,12 @@ proc newLocation*(window: Window): Location =
   let location = Location(window: window)
   let ctx = window.jsctx
   if ctx != nil:
-    let val = toJS(ctx, location)
-    let valueOf = ctx.getOpaque().valRefs[jsvObjectPrototypeValueOf]
-    defineProperty(ctx, val, "valueOf", JS_DupValue(ctx, valueOf))
-    defineProperty(ctx, val, "toPrimitive", JS_UNDEFINED)
+    let val = ctx.toJS(location)
+    let valueOf0 = ctx.getOpaque().valRefs[jsvObjectPrototypeValueOf]
+    let valueOf = JS_DupValue(ctx, valueOf0)
+    doAssert ctx.defineProperty(val, "valueOf", valueOf) != dprException
+    doAssert ctx.defineProperty(val, "toPrimitive",
+      JS_UNDEFINED) != dprException
     #TODO [[DefaultProperties]]
     JS_FreeValue(ctx, val)
   return location
@@ -3318,8 +3328,12 @@ proc selectedOptions(ctx: JSContext; this: HTMLSelectElement): JSValue
     childonly = false
   ))
   let this = ctx.toJS(this)
-  ctx.definePropertyCW(this, "selectedOptions",
+  case ctx.definePropertyCW(this, "selectedOptions",
     JS_DupValue(ctx, selectedOptions))
+  of dprException:
+    JS_FreeValue(ctx, this)
+    return JS_EXCEPTION
+  else: discard
   JS_FreeValue(ctx, this)
   return selectedOptions
 
@@ -3441,7 +3455,11 @@ proc tBodies(ctx: JSContext; this: HTMLTableElement): JSValue {.jsfget.} =
     childonly = true
   ))
   let this = ctx.toJS(this)
-  ctx.definePropertyCW(this, "tBodies", JS_DupValue(ctx, tBodies))
+  case ctx.definePropertyCW(this, "tBodies", JS_DupValue(ctx, tBodies))
+  of dprException:
+    JS_FreeValue(ctx, this)
+    return JS_EXCEPTION
+  else: discard
   JS_FreeValue(ctx, this)
   return tBodies
 
@@ -3561,7 +3579,11 @@ proc cells(ctx: JSContext; this: HTMLTableRowElement): JSValue {.jsfget.} =
     childonly = true
   ))
   let this = ctx.toJS(this)
-  ctx.definePropertyCW(this, "cells", JS_DupValue(ctx, cells))
+  case ctx.definePropertyCW(this, "cells", JS_DupValue(ctx, cells))
+  of dprException:
+    JS_FreeValue(ctx, this)
+    return JS_EXCEPTION
+  else: discard
   JS_FreeValue(ctx, this)
   return cells
 
@@ -6207,10 +6229,10 @@ return option;
   doAssert JS_SetConstructorBit(ctx, imageFun, true)
   doAssert JS_SetConstructorBit(ctx, optionFun, true)
   let jsWindow = JS_GetGlobalObject(ctx)
-  ctx.definePropertyCW(jsWindow, "Image", imageFun)
-  ctx.definePropertyCW(jsWindow, "Option", optionFun)
-  ctx.definePropertyCW(jsWindow, "HTMLDocument",
-    JS_GetPropertyStr(ctx, jsWindow, "Document"))
+  doAssert ctx.definePropertyCW(jsWindow, "Image", imageFun) != dprException
+  doAssert ctx.definePropertyCW(jsWindow, "Option", optionFun) != dprException
+  doAssert ctx.definePropertyCW(jsWindow, "HTMLDocument",
+    JS_GetPropertyStr(ctx, jsWindow, "Document")) != dprException
   JS_FreeValue(ctx, jsWindow)
 
 # Forward declaration hack
diff --git a/src/html/jsintl.nim b/src/html/jsintl.nim
index 403c6311..c9260932 100644
--- a/src/html/jsintl.nim
+++ b/src/html/jsintl.nim
@@ -241,5 +241,5 @@ proc addIntlModule*(ctx: JSContext) =
   let intl = JS_NewObject(ctx)
   ctx.registerType(NumberFormat, namespace = intl)
   ctx.registerType(PluralRules, namespace = intl)
-  ctx.defineProperty(global, "Intl", intl)
+  doAssert ctx.defineProperty(global, "Intl", intl) != dprException
   JS_FreeValue(ctx, global)
diff --git a/src/html/script.nim b/src/html/script.nim
index 9eaa9d7e..d0a8e43a 100644
--- a/src/html/script.nim
+++ b/src/html/script.nim
@@ -147,8 +147,9 @@ proc setImportMeta*(ctx: JSContext; funcVal: JSValue; isMain: bool) =
   let m = cast[JSModuleDef](JS_VALUE_GET_PTR(funcVal))
   let moduleNameAtom = JS_GetModuleName(ctx, m)
   let metaObj = JS_GetImportMeta(ctx, m)
-  definePropertyCWE(ctx, metaObj, "url", JS_AtomToValue(ctx, moduleNameAtom))
-  definePropertyCWE(ctx, metaObj, "main", false)
+  doAssert ctx.definePropertyCWE(metaObj, "url",
+    JS_AtomToValue(ctx, moduleNameAtom)) == dprSuccess
+  doAssert ctx.definePropertyCWE(metaObj, "main", false) == dprSuccess
   JS_FreeValue(ctx, metaObj)
   JS_FreeAtom(ctx, moduleNameAtom)
 
@@ -181,8 +182,8 @@ proc defineConsts*(ctx: JSContext; classid: JSClassID; consts: typedesc[enum]) =
   # it isn't...
   for e in consts:
     let s = $e
-    ctx.definePropertyE(proto, s, uint16(e))
-    ctx.definePropertyE(ctor, s, uint16(e))
+    doAssert ctx.definePropertyE(proto, s, uint16(e)) == dprSuccess
+    doAssert ctx.definePropertyE(ctor, s, uint16(e)) == dprSuccess
   JS_FreeValue(ctx, proto)
 
 proc identity(ctx: JSContext; this_val: JSValue; argc: cint;
diff --git a/src/local/client.nim b/src/local/client.nim
index 74472e75..e19a646a 100644
--- a/src/local/client.nim
+++ b/src/local/client.nim
@@ -173,9 +173,9 @@ proc newClient*(config: Config; forkserver: ForkServer; loaderPid: int;
   )
   client.attrsp = addr client.pager.term.attrs
   client.timeouts = client.pager.timeouts
-  let global = JS_GetGlobalObject(jsctx)
   jsctx.setGlobal(client)
-  jsctx.definePropertyE(global, "cmd", config.cmd.jsObj)
+  let global = JS_GetGlobalObject(jsctx)
+  doAssert jsctx.definePropertyE(global, "cmd", config.cmd.jsObj) != dprException
   JS_FreeValue(jsctx, global)
   config.cmd.jsObj = JS_NULL
   let windowCID = client.addJSModules(jsctx)