CLJS Function clobbering js function of same name

[Sam Ritchie]

Hey guys,

I ran into this last night when trying to port some ancient JS in our project over to cljs. I was defining an om component called "users-typeahead", and in the (did-mount ...) implementation calling a bare javascript function called js/users_typeahead. At the repl, the latter worked great; INSIDE the did-mount implementation, js/users_typeahead resolved to a reference to the enclosing function itself.

My expectation was that, with Clojurescript's namespacing, js/func_name would always resolve to the top-level global namespace. Instead,

Here's a minimal reproduction:

(.log js/console "Hi!")
;; logs "Hi!"

(defn console [s] (.log js/console s))

(console "Hi!")
;; throws Compilation error: TypeError: undefined is not a function

What do you think? Expected behavior, or just an edge case to avoid?

--
Sam Ritchie (@sritchie)

Paddleguru Co-Founder
703.863.8561
www.paddleguru.com
Twitter // Facebook

[David Nolen]

js/foo does not resolve to the global namespace.

David

> --
> You received this message because you are subscribed to the Google
> Groups "Clojure" group.
> To post to this group, send email to clo...@googlegroups.com
> Note that posts from new members are moderated - please be patient with your
> first post.
> To unsubscribe from this group, send email to
> clojure+u...@googlegroups.com
> For more options, visit this group at
> http://groups.google.com/group/clojure?hl=en
> ---
> You received this message because you are subscribed to the Google Groups
> "Clojure" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to clojure+u...@googlegroups.com.
> For more options, visit https://groups.google.com/d/optout.

[Herwig Hochleitner]

To clarify on David's comment, js/console puts a literal 'console' identifier into the generated js.

In your example, js/console is shadowed, because (defn console[] js/console) is emitted as ns.console = (function console(){return console;});

The discussion to be had here is whether the (internal) name of the generated js function should e.g. be mangled like let-binding:

ns.console = (function console_1337(){return console;});

Ticket?

[David Nolen]

Ah hm, yeah I'm surprised this isn't caught by the existing shadowing
and :js-globals logic. Sure open a ticket, patch welcome of course.

David

[Herwig Hochleitner]

Ticket'd http://dev.clojure.org/jira/browse/CLJS-833

[Thomas Heller]

FWIW I was not able to reproduce this behavior? What exactly does your code look like?

See the source and the compiled version:

https://gist.github.com/thheller/4731f682665d38b1053c

Seems to me the shadowing kicked in correctly.

"eval" seems to be missing in :js-globals though.

https://github.com/clojure/clojurescript/blob/e7f3a5d70a11257d5b84f1562ac01b3a4379e6e4/src/clj/cljs/analyzer.clj#L235

Cheers,

/thomas

[Herwig Hochleitner]

That's curious, here's my console interaction:

% git remote show origin 

* remote origin

  Fetch URL: git://github.com/clojure/clojurescript.git

  Push  URL: git://github.com/clojure/clojurescript.git

% git fetch origin && git checkout -b master origin/master                   

Branch master set up to track remote branch master from origin.

Switched to a new branch 'master'

% rm -rf out && ./script/clean && ./script/bootstrap

Fetching ...

[Bootstrap Completed]

% git am < 0001-CLJS-833-Test-for-fn-name-shadowing.patch

Applying: CLJS-833 Test for fn-name shadowing

% ./script/test

out/core-advanced-test.js:1333:205 Error: Assert failed: (identical? js js/eval)

% cat out/cljs/core_test.js | tail -n 11 | head -n1

var eval_7329 = (function eval(){return new cljs.core.PersistentArrayMap(null, 2, [cljs.core.constant$keyword$133,eval,cljs.core.constant$keyword$105,eval], null);

----------------------

As you can see, I start from a freshly cleaned cljs checkout and apply the test patch from the ticket. In the last line, you can see that it is indeed called (function eval(){}).

My latest two entries from % git log are (after applying my test-case patch):

commit d8d8f58df51db7246a2c92f1f863cff502dbcc45

Author: Herwig Hochleitner <her...@bendlas.net>

Date:   Mon Aug 4 11:08:12 2014 +0200

    CLJS-833 Test for fn-name shadowing

commit aa048ad64743ef51538c55163b86a90a02a8940d

Merge: 7de3636 bb3adca

Author: David Nolen <david...@gmail.com>

Date:   Sat Aug 2 10:45:54 2014 -0400

    Merge branch 'master' of github.com:clojure/clojurescript

Could you please confirm, that you can't reproduce the issue with the latest master from github, or point me to where I'm doing something stupid?

kind regards

[Herwig Hochleitner]

I dug a bit deeper to see where :js-globals came from and found the old ticket for this exact issue: http://dev.clojure.org/jira/browse/CLJS-680

I propose that we remove it, because it's unnessecary when we gensym fn names the same way as let bindings + blacklists are never a great solution, but in the case of an open set of names (such as globals that can differ from runtime to runtime), they are particularly awful.

Ad reproducing the issue: Can the shadowing mechanism be influenced by compiler flags such as :advanced?

[Thomas Heller]

I was not able to reproduce Sam's initial problem.

"eval" as I said does not appear in the :js-globals which seems like a mistake.

[Nicola Mometto]

Try with (fn document [] js/document)

[David Nolen]

On Mon, Aug 4, 2014 at 9:11 AM, Herwig Hochleitner
<hhochl...@gmail.com> wrote:
> I dug a bit deeper to see where :js-globals came from and found the old
> ticket for this exact issue: http://dev.clojure.org/jira/browse/CLJS-680
> I propose that we remove it, because it's unnessecary when we gensym fn
> names the same way as let bindings + blacklists are never a great solution,
> but in the case of an open set of names (such as globals that can differ
> from runtime to runtime), they are particularly awful.

We don't gensym let bindings - the shadowing logic was added to create
stable names. This will be useful if anyone wants to create more
powerful debuggers for ClojureScript. That said for self reference
using an implicitly shadowed name would seem to avoid these issues,
i.e. console__0.

> Ad reproducing the issue: Can the shadowing mechanism be influenced by
> compiler flags such as :advanced?

Not that I'm aware of.

[Herwig Hochleitner]

2014-08-04 16:27 GMT+02:00 David Nolen <dnolen...@gmail.com>:

We don't gensym let bindings - the shadowing logic was added to create
stable names.

In fact, we do within statement contexts. I wrote that as a fix for the case

(do (let [x 1] (fn [] x)) 

    (let [x 2] (comment clobbered x in fn)))

(as well as toplevel lets) as part of CLJS-401 and it is still in place: https://github.com/clojure/clojurescript/commit/234bd3a85213d98595090b3a6625f0aeae7ea6ed

Im sure that this could be subsumed by your work on stable renaming (which it pre-dated), but then the debugger issue would need additional work, see below. 

This will be useful if anyone wants to create more
powerful debuggers for ClojureScript. That said for self reference
using an implicitly shadowed name would seem to avoid these issues,
i.e. console__0.

Point taken. Let me sidetrack a bit on the gensymming I introduced, in order to aid a hypothetical effort seeking to remove it.

I get that we would like a debugger to predict the generated js names from a piece of cljs source without access to the original analyzer environment.

Premise: Would you agree that such a mechanism only makes sense if it works with isolated toplevel forms, a la REPL?

I.E. the debugger shouldn't need to reanalyze the whole program to get at a given name assignment, but just the toplevel form at hand. That makes sense, right?

Then, even with your scheme of keeping a separate gensym counter for every var name, there's a choice between fire and frying pan:

1) Wrap (at least) toplevel lets into (function(){..}()) (IIRC, David, you rejected this approach to CLJS-401 because of the "minimal wrapping" philosophy)

2) Keep the initial counters for each (toplevel) form in some kind of source map, e.g. code comments

   Given that js 1.5 has no direct let-equivalent, every toplevel form closing over some var 'x' will need to know about every previous toplevel form closing over another var 'x' (with the same name).

   So this is pretty much on par with gensymming, in terms of effort required to book-keep var names.

In essence, removing my gensym and going with just the predictable counters means that you can't reset the counters when a let closes (only when an fn closes).

So in general because a let can occur toplevel, solution 2) means that you have to treat all sources as one giant do - statement, maybe defining explicit breaking points for the debugger via source map.

I think that this decision is fundamental i.e. TINA, but I would love to be convinced otherwise.

Given all that + the intuition that closure and/or jit eats (function(){}())s for breakfast, let me repropose to go with fire (solution 1).

Maybe a separate ticket?

kind regards

[Thomas Heller]

https://gist.github.com/thheller/4731f682665d38b1053c

[Herwig Hochleitner]

Thomas, in my test, (defn console []) is munged aswell, so I'm guessing that Sam is using a version from before https://github.com/clojure/clojurescript/commit/f371c04d95a00cdda79c63f89f35088d62de8e73

Sam, is that correct?

The observation that eval should be in js-globals is irrelevant, because as I detailed a couple of messages ago, js-globals is wrong and should be removed. 

(fn x []) should never cast a shadow on js/x for any x.

So far, I believe David to agree. I commented on CLJS-833 to reflect this point of view.

Waiting on a statement towards fully removing gensyms from the compiler, i.e. CLJS-401 revisited ...

[Sam Ritchie]

Hey Herwig,

I'm currently on [org.clojure/clojurescript "0.0-2261"], and I'm seeing no munging:

paddleguru.api.register.validation> (defn console [])
#<function console(){return null;
}>
nil
paddleguru.api.register.validation> (defn console [s] (.log js/console s))
#<function console(s){return console.log(s);
}>
nil
paddleguru.api.register.validation> (console "HI!")
"Error evaluating:" (console "HI!") :as "paddleguru.api.register.validation.console.call(null,\"HI!\");\n"
#<TypeError: undefined is not a function>

TypeError: undefined is not a function

    at console (eval at <anonymous> (https://local.paddleguru.com/cljs/dev/generated.js:83183:294), <anonymous>:1:146)
    at eval (eval at <anonymous> (https://local.paddleguru.com/cljs/dev/generated.js:83183:294), <anonymous>:1:108)
    at eval (eval at <anonymous> (https://local.paddleguru.com/cljs/dev/generated.js:83183:294), <anonymous>:5:3)
    at https://local.paddleguru.com/cljs/dev/generated.js:83183:289
    at https://local.paddleguru.com/cljs/dev/generated.js:83197:4
    at G__30641__2 (https://local.paddleguru.com/cljs/dev/generated.js:23732:22)
    at G__30641 [as call] (https://local.paddleguru.com/cljs/dev/generated.js:23977:28)
    at null.<anonymous> (https://local.paddleguru.com/cljs/dev/generated.js:83238:80)
    at goog.events.EventTarget.fireListeners (https://local.paddleguru.com/cljs/dev/generated.js:42772:23)
    at Function.goog.events.EventTarget.dispatchEventInternal_ (https://local.paddleguru.com/cljs/dev/generated.js:42817:26)
nil

Herwig Hochleitner

August 4, 2014 at 11:43 AM

--
You received this message because you are subscribed to the Google
Groups "Clojure" group.
To post to this group, send email to clo...@googlegroups.com
Note that posts from new members are moderated - please be patient with your first post.
To unsubscribe from this group, send email to
clojure+u...@googlegroups.com
For more options, visit this group at
http://groups.google.com/group/clojure?hl=en
---
You received this message because you are subscribed to the Google Groups "Clojure" group.
To unsubscribe from this group and stop receiving emails from it, send an email to clojure+u...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Thomas Heller

August 4, 2014 at 10:40 AM

https://gist.github.com/thheller/4731f682665d38b1053c

On Monday, August 4, 2014 3:34:33 PM UTC+2, Nicola Mometto wrote: --

You received this message because you are subscribed to the Google
Groups "Clojure" group.
To post to this group, send email to clo...@googlegroups.com
Note that posts from new members are moderated - please be patient with your first post.
To unsubscribe from this group, send email to
clojure+u...@googlegroups.com
For more options, visit this group at
http://groups.google.com/group/clojure?hl=en
---
You received this message because you are subscribed to the Google Groups "Clojure" group.
To unsubscribe from this group and stop receiving emails from it, send an email to clojure+u...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Nicola Mometto

August 4, 2014 at 7:34 AM

Try with (fn document [] js/document)

Thomas Heller

August 4, 2014 at 7:30 AM

I was not able to reproduce Sam's initial problem.

"eval" as I said does not appear in the :js-globals which seems like a mistake.

--
You received this message because you are subscribed to the Google
Groups "Clojure" group.
To post to this group, send email to clo...@googlegroups.com
Note that posts from new members are moderated - please be patient with your first post.
To unsubscribe from this group, send email to
clojure+u...@googlegroups.com
For more options, visit this group at
http://groups.google.com/group/clojure?hl=en
---
You received this message because you are subscribed to the Google Groups "Clojure" group.
To unsubscribe from this group and stop receiving emails from it, send an email to clojure+u...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Herwig Hochleitner

August 4, 2014 at 7:11 AM

I dug a bit deeper to see where :js-globals came from and found the old ticket for this exact issue: http://dev.clojure.org/jira/browse/CLJS-680

I propose that we remove it, because it's unnessecary when we gensym fn names the same way as let bindings + blacklists are never a great solution, but in the case of an open set of names (such as globals that can differ from runtime to runtime), they are particularly awful.

Ad reproducing the issue: Can the shadowing mechanism be influenced by compiler flags such as :advanced?

--
You received this message because you are subscribed to the Google
Groups "Clojure" group.
To post to this group, send email to clo...@googlegroups.com
Note that posts from new members are moderated - please be patient with your first post.
To unsubscribe from this group, send email to
clojure+u...@googlegroups.com
For more options, visit this group at
http://groups.google.com/group/clojure?hl=en
---
You received this message because you are subscribed to the Google Groups "Clojure" group.
To unsubscribe from this group and stop receiving emails from it, send an email to clojure+u...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[David Nolen]

I would rely on the behavior in the REPL to check this - the REPL compilation environment is likely different.

[David Nolen]

I meant would not

[Luc Prefontaine]

That's what I inferred but it has nothing to do with my astonishing ESP capabilities,
currently drinking an excellent beer in Rabat :)

Cheers,

Luc P

> >> Herwig Hochleitner <hhochl...@gmail.com>

> >> Thomas Heller <th.h...@gmail.com>

> >> August 4, 2014 at 10:40 AM
> >> https://gist.github.com/thheller/4731f682665d38b1053c
> >>
> >> On Monday, August 4, 2014 3:34:33 PM UTC+2, Nicola Mometto wrote: --
> >> You received this message because you are subscribed to the Google
> >> Groups "Clojure" group.
> >> To post to this group, send email to clo...@googlegroups.com
> >> Note that posts from new members are moderated - please be patient with
> >> your first post.
> >> To unsubscribe from this group, send email to
> >> clojure+u...@googlegroups.com
> >> For more options, visit this group at
> >> http://groups.google.com/group/clojure?hl=en
> >> ---
> >> You received this message because you are subscribed to the Google Groups
> >> "Clojure" group.
> >> To unsubscribe from this group and stop receiving emails from it, send an
> >> email to clojure+u...@googlegroups.com.
> >> For more options, visit https://groups.google.com/d/optout.

> >> Nicola Mometto <brob...@gmail.com>

> >> August 4, 2014 at 7:34 AM
> >> Try with (fn document [] js/document)
> >>
> >>

> >> Thomas Heller <th.h...@gmail.com>

> >> August 4, 2014 at 7:30 AM
> >> I was not able to reproduce Sam's initial problem.
> >>
> >> "eval" as I said does not appear in the :js-globals which seems like a
> >> mistake.
> >> --
> >> You received this message because you are subscribed to the Google
> >> Groups "Clojure" group.
> >> To post to this group, send email to clo...@googlegroups.com
> >> Note that posts from new members are moderated - please be patient with
> >> your first post.
> >> To unsubscribe from this group, send email to
> >> clojure+u...@googlegroups.com
> >> For more options, visit this group at
> >> http://groups.google.com/group/clojure?hl=en
> >> ---
> >> You received this message because you are subscribed to the Google Groups
> >> "Clojure" group.
> >> To unsubscribe from this group and stop receiving emails from it, send an
> >> email to clojure+u...@googlegroups.com.
> >> For more options, visit https://groups.google.com/d/optout.

> >> Herwig Hochleitner <hhochl...@gmail.com>

> >> Twitter <http://twitter.com/paddleguru> // Facebook
> >> <http://facebook.com/paddleguru>

Luc Prefontaine<lprefo...@softaddicts.ca> sent by ibisMail!

[Herwig Hochleitner]

2014-08-04 22:28 GMT+02:00 David Nolen <dnolen...@gmail.com>:

I meant would not

On Mon, Aug 4, 2014 at 4:28 PM, David Nolen <dnolen...@gmail.com> wrote:

I would rely on the behavior in the REPL to check this - the REPL compilation environment is likely different.

Yes, I noticed that ./script/repl has different behavior from ./script/test on this. I assume that this is because the repl-env has no :js-globals, no?