<< Back to previous view

[CLJS-1440] Allow eval to work inside a web worker Created: 01/Sep/15  Updated: 02/Sep/15  Resolved: 02/Sep/15

Status: Closed
Project: ClojureScript
Component/s: None
Affects Version/s: 1.7.48
Fix Version/s: None

Type: Enhancement Priority: Major
Reporter: Zach Oakes Assignee: Unassigned
Resolution: Completed Votes: 0
Labels: None
Environment:

Google Chrome 44.0


Attachments: Text File cljs_1440.patch     Text File cljs_1440.patch    

 Description   

Evaling ClojureScript code inside a web worker fails with `window is not defined`. I will attach a patch.



 Comments   
Comment by Francis Avila [ 01/Sep/15 5:13 PM ]

Maybe goog/global is better? It will always be the global object no matter what the platform calls it.

Comment by David Nolen [ 01/Sep/15 5:15 PM ]

Agreed that goog.global is preferred.

Comment by Zach Oakes [ 01/Sep/15 5:55 PM ]

I tried goog/global and it works. I attached a new patch.

Comment by David Nolen [ 02/Sep/15 6:08 AM ]

I went ahead and applied this, Zach may sure to submit your CA. Thanks.

Comment by David Nolen [ 02/Sep/15 6:09 AM ]

fixed https://github.com/clojure/clojurescript/commit/1b7390450243693d0b24e8d3ad085c6da4eef204

Comment by Zach Oakes [ 02/Sep/15 7:32 AM ]

Yep I submitted the CA yesterday.

Comment by David Nolen [ 02/Sep/15 8:06 AM ]

Sweet, thanks much!





[CLJS-1439] Add type annotations to goog-define defined vars Created: 01/Sep/15  Updated: 01/Sep/15

Status: Open
Project: ClojureScript
Component/s: None
Affects Version/s: None
Fix Version/s: None

Type: Enhancement Priority: Major
Reporter: Martin Klepsch Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None


 Description   

Currently it's still required to annotate goog-define constants with ^boolean to allow the Closure compiler to safely remove dead branches. The macro could emit the var with ^boolean metadata to make this unnecessary.

In general it would be nice to have similar annotations for the already defined constants like goog.DEBUG although I'm not sure how/if that's possible.






[CLJS-1438] If .startsWith (or any other method) is used on a clojurescript string object when using :optimizarions :advanced code does not work Created: 31/Aug/15  Updated: 01/Sep/15  Resolved: 31/Aug/15

Status: Resolved
Project: ClojureScript
Component/s: None
Affects Version/s: 1.7.48
Fix Version/s: None

Type: Defect Priority: Major
Reporter: Felipe Gerard Assignee: Unassigned
Resolution: Declined Votes: 0
Labels: compiler, interop
Environment:

MAC OS


Attachments: File minimal.tgz    
Waiting On: Alex Miller

 Description   

This is a minimal project HelloWorld (I am attaching a full project "minimal.tgz"):
(ns minimal.core)

(let [elem (.getElementById js/document "app")]
(set! (.-innerHTML elem) (str "Hello World " (.. "Hello" (startsWith "H")))))

Will run fine with no optimizarions but when using :advanced I get: Uncaught TypeError: "Hello".Lb is not a function



 Comments   
Comment by David Nolen [ 31/Aug/15 3:31 PM ]

This is not a bug. That's how Closure advanced optimization works. Use goog.string/startsWith instead.

Comment by Rohit Aggarwal [ 01/Sep/15 5:37 AM ]

I think that the issue is that Closure compiler may not be including ES2015 externs yet. `.startsWith` has been added to String prototype in ES2015. So I would have imagined that if the extern was provided, it shouldn't have mangled the function name.





[CLJS-1437] Recompile dependents broken when files are not inside one directory Created: 30/Aug/15  Updated: 02/Sep/15

Status: Open
Project: ClojureScript
Component/s: None
Affects Version/s: 1.7.48
Fix Version/s: Next

Type: Defect Priority: Major
Reporter: Juho Teperi Assignee: Juho Teperi
Resolution: Unresolved Votes: 0
Labels: build

Attachments: Text File compile-dependency-order.patch    

 Description   

When a file is changed files dependent on that file are marked for recompile. This doesn't seem to work when the changed file is not in the directory which is passed to build and the directory where dependent namespace is.

Looks like this caused by how build and compile-file work:

  • Build calls -compile -> compile-dir -> compile-root which sorts the files in dependency order and calls -compile for the files in dependency order
    • core ns is compiled
  • Build calls add-depedencies -> add-dependencies -> get-compiled-cljs -> -compile -> compile-file
    • test ns is compiled, core ns is marked as dependent on test ns but as core ns is already compiled this doesn't matter

I believe this effectively breaks :recompile-dependents in cases all cases but one where all the sources are inside one directory. In multiple directory case order of directories passed to cljs.build.api/inputs affects the order on which compile is called so it's possible that some dependent namespaces are compiled before namespaces they depend on are compiled.

Original problem was that added tests were not picked up by run-tests but I have since reproduced this without cljs.test.

The minimal way to reproduce this needs two namespaces, where one depends on another. To display that the dependent namespace is not recompiled, it will print some compile time metadata from the another namespace.

src/hello/core.cljs

(ns hello.core
  (:require hello-world.test))

(enable-console-print!)

(println (:foo (meta #'hello.test/a)))

src2/hello/test.cljs

(ns hello.test)

(def ^{:foo :bar} a "foobar")

To build:

(require '[cljs.build.api :refer [build]])

(def opts {:main "hello.core"
           :output-to "out/main.js"
           :output-dir "out"
           :optimizations :none
           :verbose true})

(defn broken []
  (build "src" opts))

(defn working []
  " Note: ordering of the dirs matters
  (build (inputs "src2" "src" opts)))

To trigger the problem, the metadata needs to be changed after running the build once. Changing test ns doesn't cause core ns to recompiled and changing the metadata doesn't have any effect to println on core ns.



 Comments   
Comment by Juho Teperi [ 02/Sep/15 5:55 PM ]

First version for review.

Still missing support for finding sources for any build source parameter. Previously this was provided through Compilable interface but that doesn't make sense anymore as we don't want to compile them but find the namespaces.

Function naming could use some thinking.

This includes minimally changed alternative build function: build-new. I think future improvements to recompile-dependents would be limited to compile-sources where it would be possible to mark all dependent namespaces for recompile.





[CLJS-1436] self-host: Doesn't load dep ns when loading own cache Created: 29/Aug/15  Updated: 02/Sep/15

Status: Open
Project: ClojureScript
Component/s: None
Affects Version/s: 1.7.48
Fix Version/s: Next

Type: Defect Priority: Major
Reporter: Mike Fikes Assignee: David Nolen
Resolution: Unresolved Votes: 0
Labels: bootstrap

Attachments: Text File CLJS-1436-1.patch     Text File CLJS-1436-2.patch    
Patch: Code

 Description   

Background: Both *load-fn* and *eval-fn* support the concept of passing JavaScript and analysis cache info. Such data passed to *eval-fn* should be able to be stored and subsequently passed to *load-fn* upon subsequent runs in order to effect analysis/compilation caching.

If one namespace depends on another then transitive loading occurs if no caching is involved. But if cached information it returned to *load-fn* when the top-level namespace is loaded, then transitive loading does not occur.

My speculation is that this is the case because, while the analysis cache is incorporated here

https://github.com/clojure/clojurescript/blob/r1.7.122/src/main/cljs/cljs/js.cljs#L206-L208

nothing further is done to load dependent namespaces as is done when ClojureScript source is passed in the callback.

Here is a concrete example. Let foo.core depend on bar.core:

(ns foo.core
  (:require bar.core))

(prn bar.core/x)
(ns bar.core)

(def x 2)

If you require foo.core (via an ns form passed to eval-str), then *load-fn* is called with

{:name foo.core, :macros nil, :path "foo/core"}

and it is called back upon with the loaded ClojureScript source:

{:lang :clj, :source "(ns foo.core\n  (:require bar.core))\n\n(prn bar.core/x)\n"}

and the embedded :require then causes *load-fn* to be invoked for bar.core:

{:name bar.core, :macros nil, :path "bar/core"}

with its source being passed to the callback:

{:lang :clj, :source "(ns bar.core)\n\n(def x 2)\n"}

Then the evaluation phase begins with two evaluations (done properly in reverse dependency order):

{:lang :clj, :name bar.core, :path "bar/core", :source "goog.provide(\"bar.core\");\nbar.core.x = 2;\n", :cache {:use-macros nil, :excludes #{}, :name bar.core, :imports nil, :requires nil, :uses nil, :defs {x {:name bar.core/x, :file nil, :line 3, :column 1, :end-line 3, :end-column 7, :meta {:file bar.core, :line 3, :column 6, :end-line 3, :end-column 7}}}, :require-macros nil, :doc nil}}

and

{:lang :clj, :name foo.core, :path "foo/core", :source "goog.provide(\"foo.core\");\ncljs.core.prn.call(null,bar.core.x);\n", :cache {:name foo.core, :doc nil, :excludes #{}, :use-macros nil, :require-macros nil, :uses nil, :requires {bar.core bar.core}, :imports nil}}

which causes 2 to be printed.

Then if you start fresh, but go through the same sequence, consuming cached information, you start off the same with *load-fn* being called the same way:

{:name foo.core, :macros nil, :path "foo/core"}

and with the callback now being passed cached information back in return:

{:lang :js, :source "goog.provide(\"foo.core\");\ncljs.core.prn.call(null,bar.core.x);\n", :cache {:name foo.core, :doc nil, :excludes #{}, :use-macros nil, :require-macros nil, :uses nil, :requires {bar.core bar.core}, :imports nil}}

This JavaScript is evaluated by cljs.js here

https://github.com/clojure/clojurescript/blob/r1.7.122/src/main/cljs/cljs/js.cljs#L205

and this causes this to be printed

Can't find variable: bar

Perhaps this isn't a defect if instead it is expected that clients of cljs.js handle the loading of dependent namespaces when caching is involved.



 Comments   
Comment by David Nolen [ 30/Aug/15 10:34 AM ]

A patch for this is welcome. Analysis caches for a ns include all the information needed to load lib and macro dependencies.

Comment by Mike Fikes [ 02/Sep/15 3:55 PM ]

This appears to be a fairly straightforward patch, but attached it with a -1 revision indicator in case it ultimately needs a few rounds of review. (IMHO, there is definitely no urgency to review this, as it is a self-host ticket.)

Comment by David Nolen [ 02/Sep/15 4:51 PM ]

Lets break out helper functions for this one please. Thanks!

Comment by Mike Fikes [ 02/Sep/15 9:11 PM ]

Broke out with a few explicitly named helper functions. Let me know if I took it too far. Also, feel free to simply modify this to suit.





[CLJS-1435] self-host: lexical scope is broken Created: 29/Aug/15  Updated: 29/Aug/15  Resolved: 29/Aug/15

Status: Closed
Project: ClojureScript
Component/s: None
Affects Version/s: 1.7.48
Fix Version/s: Next

Type: Defect Priority: Major
Reporter: Chris Truter Assignee: Unassigned
Resolution: Completed Votes: 0
Labels: bootstrap

Attachments: Text File 1435.patch    
Patch: Code and Test

 Description   

This has the cause as http://dev.clojure.org/jira/browse/CLJS-1400, but demonstrates a more provocative compilation error.

Sample code using cljs.js to demonstrate the failure:

(cljs/eval
  (cljs/empty-state)
  '(let [x 1]
     (let [x (inc x)]
       (prn x))
     (prn x))
  {:eval (juxt (comp prn :source) cljs/js-eval)}
  (fn [& _]))

Which displays both the code (line breaks unescaped here for convenience) and resulting error:

"var x_22 = 1;
 var x_23__$1 = (x_23 + 1);
 cljs.core.prn.call(null,x_23__$1);
 cljs.core.prn.call(null,x_22);"
VM1654:2 Uncaught ReferenceError: x_23 is not defined

This issue is more noticeable than in the case encountered via "doseq", as the naming error occurs in a read position rather than write, so we get an error instead of unexpected behaviour.

The patch attached fixies this and 1400



 Comments   
Comment by Chris Truter [ 29/Aug/15 10:32 AM ]

Attached patch with a (hopefully) faster alternate

Comment by David Nolen [ 29/Aug/15 11:00 AM ]

fixed https://github.com/clojure/clojurescript/commit/ed01ddf9a87eb67aa50feacd72e8aa66083cdbfc





[CLJS-1434] clojure.walk no longer preserves meta on vectors Created: 29/Aug/15  Updated: 29/Aug/15  Resolved: 29/Aug/15

Status: Closed
Project: ClojureScript
Component/s: None
Affects Version/s: 1.7.48
Fix Version/s: Next

Type: Defect Priority: Major
Reporter: David Nolen Assignee: David Nolen
Resolution: Completed Votes: 0
Labels: None


 Description   
(require '[clojure.walk :as walk])
(-> (walk/prewalk 
      identity [1 (with-meta [1 2] {:foo 1})])
  (nth 1)
  meta) ;; nil instead of {:foo 1}


 Comments   
Comment by David Nolen [ 29/Aug/15 7:15 AM ]

The problem is that there is an IMapEntry case which will capture vectors and not preserve their meta. But the IMapEntry case is redundant anyway as as map entries are just vectors and these will be handled by the coll? branch of the switch.

Comment by David Nolen [ 29/Aug/15 7:18 AM ]

fixed https://github.com/clojure/clojurescript/commit/1e626ddeeebb748701e113e60032f244209de589





Generated at Thu Sep 03 03:54:00 CDT 2015 using JIRA 4.4#649-r158309.