Fork me on GitHub
Slack Archive

This page is not created by, affiliated with, or supported by Slack Technologies, Inc.

2019-02-25

Channels

#cljs-dev
<
2019-02-25
>
mhuebert10:02:07

I’ve been digging through the emitted code for various js-interop examples, and was surprised to find that CLJS does not seem to “know’ that goog.object/containsKey returns a boolean value, and so when it is used in an if statement it is wrapped in a truth_ check unless one adds a type hint. But thinking about it, I suppose it makes sense that there is no upward flow of type information from js -> cljs

darwin10:02:31

what if you wrap it with a cljs function which does explicit type hints, would it ruin advanced compilation inlining?

mhuebert11:02:58

👍:skin-tone-2: looks like it compiles to identical code

mhuebert11:02:15

(defn ^boolean containsKey [obj k]
  (gobj/containsKey obj k))
^ compiles to the inlined output of gobj/containsKey without wrapping in truth_

mfikes11:02:17

FWIW, gleaning type hint information from Closure is an idea that has been discussed (worth pursuing). See this bit of logic David worked out https://github.com/clojure/clojurescript/blob/f97d766defd02f7d43abd37e3e9b04790a521b1e/src/main/clojure/cljs/externs.clj#L165-L172

mhuebert11:02:05

interesting. does look promising to me

mhuebert11:02:29

is there some kind of special handling of the goog.reflect namespace that changes how it is included/required? if I emit (~'goog.reflect/objectProperty ...) from a macro and use it from another namespace, I get warnings like 

WARNING: No such namespace: goog.reflect, could not locate goog/reflect.cljs, goog/reflect.cljc, or JavaScript source providing "goog.reflect" at line 21 /../js-interop/src/test/applied_science/js_interop_usage.cljs
WARNING: Use of undeclared Var goog.reflect/objectProperty at line 21 /../js-interop/src/test/applied_science/js_interop_usage.cljs
(despite properly requiring goog.reflect in the cljs namespace which self-requires its same-named macro ns) as a workaround I am using js/goog.reflect.objectProperty instead, but when compiling in :advanced mode that results in warnings related to externs inference: 
WARNING: out/test-2/inferred_externs.js:14: WARNING - name goog is not defined in the externs.
goog.reflect;
^^^^

Feb 25, 2019 12:25:31 PM com.google.javascript.jscomp.LoggerErrorManager println
WARNING: out/test-2/inferred_externs.js:15: WARNING - name goog is not defined in the externs.
goog.reflect.objectProperty;
^^^^

thheller12:02:35

if you just emit a direct call to (goog.reflect/objectProperty ...) in the macro it should work?

mhuebert12:02:38

that is the workaround, and results in the 2nd kind of warnings

mhuebert12:02:50

’goog.reflect/objectProperty results in the 1st kind of warnings

thheller12:02:00

yes, I mean the def of the symbol in the first place

mhuebert12:02:24

oh. why would that cause a problem? I thought (~'some-symbol) would be equivalent to (def x 'some-symbol) ~some-symbol. at least it seems to behave that way everywhere else

thheller12:02:24

does the warning go away if you use goog.reflect/objectProperty here directly instead of the ~reflect-property? https://github.com/appliedsciencestudio/js-interop/blob/master/src/main/applied_science/js_interop.clj#L41

thheller12:02:04

not actually sure. I would expect it to work as well but unquote is kinda weird

mhuebert12:02:36

yeah it behaves the same if i write (~'goog.reflect/objectProperty ~(dot-name k) ~obj)

thheller12:02:47

no without ~'

thheller12:02:08

just a normal fully qualified symbol

mhuebert12:02:16

that breaks

mhuebert12:02:27

or rather warns

mhuebert12:02:40

but that i would expect because goog.reflect doesn’t exist for the clj namespace

mhuebert12:02:58

ie. i also can’t call a function from the cljs namespace without using a quoted fully qualified symbol

thheller12:02:26

(defn wrap-key
  "Convert key to string at compile time when possible."
  ([k]
   (wrap-key k nil))
  ([k obj]
   (cond
     (or (string? k)
         (number? k)) k
     (keyword? k) (name k)
     (symbol? k) (cond (= (:tag (meta k)) "String") k
                       (dot-sym? k) `(goog.reflect/objectProperty ~(dot-name k) ~obj)
                       :else `(wrap-key ~k))
     :else `(wrap-key ~k))))

thheller12:02:30

this breaks?

mhuebert12:02:36

that warns, yes

mhuebert12:02:08

that is also why i use (def lookup-sentinel 'applied-science.js-interop/lookup-sentinel). I can’t just use lookup-sentinel because it doesn’t exist for clj

mhuebert12:02:26

and it’s ugly putting such a long fully qualified syms everywhere

mhuebert12:02:05

a very handy change in cljs macro behaviour would be to have clj not warn on vars that are defined in the same-named cljs namespace

thheller12:02:28

(ns hello-world.core
  (:require-macros [hello-world.core :refer (x)])
  (:require [goog.reflect]))

(prn (x {}))

thheller12:02:37

(ns hello-world.core)

(defmacro x [thing]
  `(goog.reflect/objectProperty "prop" ~thing))

thheller12:02:41

compiles fine. no warnings?

mhuebert12:02:02

yes, this only shows up if you require hello-world.core from a 3rd namespace which does not explicitly require goog.reflect

mhuebert12:02:14

earlier i wrote some examples comparing goog.reflect and goog.object and using the same syntax/structure with goog.object doesn’t cause any issues

thheller12:02:10

yeah I have it reproduced

mhuebert12:02:07

i wonder why it doesn’t affect goog.object

mhuebert12:02:49

oh, it’s in implicit-nses

thheller12:02:39

yeah. should be an easy fix. I'm unsure where CLJS stores the closure namespaces though, if it does at all.

dnolen16:02:53

I think the above is a good reason to push ahead on the parsing Closure namespaces

👍 10
dnolen16:02:24

names, docs, type info

thheller17:02:21

The info required for this should already be available. Just can't remember where. Parsing would be a bonus but probably a lot more work

dnolen18:02:23

externs stuff already has basic parsing, we just need to enhance it to extract the other bits