Fork me on GitHub
Slack Archive

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

2022-05-05

Channels

#cljs-dev
<
2022-05-05
>
dnolen16:05:54

@timothypratley looking at your issue, it appears this case is just missing - no test for this particular case - required JS lib fn invoke, (js/foo) is tested and works - so it will be a good fix

😎 1
dnolen17:05:41

I was doing something complicated, but then realized this can probably be fixed in the type inference part of the code only - https://github.com/clojure/clojurescript/pull/173/files

dnolen17:05:48

we'll see if the tests pass

dnolen17:05:21

@timothypratley there is one potential enhancement - infer that any usage of method where we know the first argument is ^js and the callback fn is inlined, we auto-hint the arguments to ^js

❤️ 1
dnolen17:05:00

if the argument is a var - we warn if that var did not declare all its arguments as ^js

dnolen17:05:21

opinions about this enhancement welcome

timothypratley18:05:23

IMO that would be very helpful, and AFAIK then all scenarios are covered which is awesome.

timothypratley18:05:01

Oh, just to correct myself, I don't think it would catch: 

(-> (signInWithPopup auth google-provider)
      (.then (fn [^js/firebase.firestore.UserCredential result]
               (when (seq scopes)
                 (gauth/authorize (-> result (.-user) (.-email)))))))
because here the (fn [result] ...) is called by promise instead of callback. :thinking_face:

dnolen18:05:18

it would catch this

dnolen18:05:27

because signInWithPop is ^js

dnolen18:05:37

thus .then must be a ^js invoke

dnolen18:05:42

and then we check the args

timothypratley18:05:59

That's really cool.

dnolen17:05:54

not in scope for next release - but if nobody sees any problem with that (it seems OK to me at the moment) - can come at a later time (and would be experimental/optional until enough feedback is collected, i.e. :extern-infer-js-callbacks true etc)

dnolen17:05:29

with CLJS-3373, you only need to switch to goog.object inside callback handlers and you don't need all that redundant hinting

dnolen17:05:23

CLJS-3373 fix merged

1
timothypratley18:05:31

Thank you so much David! You rock 🙂

dnolen18:05:53

the only gap w/ callback externs infer would be higher order invokes, but I think we should probably warn on those since it's potential pothole anyway

timothypratley18:05:59

My guess is that hinting ^js on a function argument would probably be fine anyway (even though it isn't necessary); I don't think it would have any effect? :thinking_face: