<< Back to previous view

[CLJS-879] Add function `update` from Clojure 1.7 Created: 31/Oct/14  Updated: 31/Oct/14  Resolved: 31/Oct/14

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

Type: Enhancement Priority: Minor
Reporter: Daniel Skarda Assignee: Unassigned
Resolution: Completed Votes: 0
Labels: None


 Comments   
Comment by David Nolen [ 31/Oct/14 2:48 AM ]

fixed https://github.com/clojure/clojurescript/commit/15fbbf5d63fbc860e0fe4f7d45c7f00a27ebc0ba

Comment by Daniel Skarda [ 31/Oct/14 3:03 AM ]

David, thanks for being so quick!

You are faster than my `git format-patch` Are not you supposed to be on tour with Hairy Sands?

Thank you for fast fix.





[CLJS-875] Compiler error on :" Created: 22/Oct/14  Updated: 31/Oct/14

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

Type: Defect Priority: Major
Reporter: Stuart Mitchell Assignee: David Nolen
Resolution: Unresolved Votes: 0
Labels: compiler, errormsgs
Environment:

Ubuntu 14.04
org.clojure/clojurescript "0.0-2277"



 Description   

(str "this will break" :"the parser")
This simple form (note the misplaced ':' causes the compiler to barf with the following error
Caused by: clojure.lang.ExceptionInfo: EOF while reading string

no mention of the ':' and in larger blocks of code the unmatched brackets several lines away is flagged

in clojure the error is
RuntimeException Invalid token: : clojure.lang.Util.runtimeException (Util.java:221)
RuntimeException Unmatched delimiter: ) clojure.lang.Util.runtimeException (Util.java:221)

Note it mentions the errant ':'



 Comments   
Comment by Stuart Mitchell [ 22/Oct/14 11:04 PM ]

I would like the error reporting from the compiler improved in this case, so that it mentions that the problem is caused by the ':'.

Comment by Nicola Mometto [ 23/Oct/14 7:53 AM ]

This appears to be an issue caused by tools.reader, I'm working on a fix for it

Comment by Nicola Mometto [ 23/Oct/14 9:17 AM ]

I just pushed a 0.8.10 release of tools.reader that fixes this issue





[CLJS-510] Problems with :optimizations :whitespace & :output-wrapper true Created: 28/May/13  Updated: 31/Oct/14

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

Type: Defect Priority: Minor
Reporter: Aku Kotkavuo Assignee: Unassigned
Resolution: Unresolved Votes: 2
Labels: None
Environment:

OS X 10.8.3, Google Chrome 28


Attachments: Text File CLJS-510.patch    

 Description   

When I have both output wrapper and :whitespace optimizations enabled, I get the following error when loading the produced output:

goog.provide("goog.string");
goog.provide("goog.string.Unicode");
goog.string.Unicode = {NBSP:"\u00a0"};
^- Uncaught TypeError: Cannot set property 'Unicode' of undefined

Any ideas on what is going wrong? The error disappears if I use :simple or :advanced optimizations or if I set :output-wrapper to false (combined with any optimization level).



 Comments   
Comment by Aku Kotkavuo [ 28/May/13 10:14 AM ]

ClojureScript version used is 0.0-1806.

Comment by Aku Kotkavuo [ 28/May/13 10:30 AM ]

I'm also seeing this:

GET http://localhost:7070/deps.js 404 (Not Found)
goog.writeScriptTag_
goog.importScript_
goog.importScript_(goog.basePath + "deps.js")

I have no idea what deps.js is, something related to Google Closure Compiler?

Comment by David Nolen [ 19/Nov/13 9:23 PM ]

Is this problem still present? Thanks.

Comment by Immo Heikkinen [ 11/Mar/14 3:59 AM ]

The problem is still present in clojurescript master. To reproduce, compile https://github.com/clojure/clojurescript/tree/master/samples/hello using

cljsc src {:output-wrapper true :optimizations :whitespace} > hello.js
Comment by Herwig Hochleitner [ 13/Oct/14 5:28 PM ]

I just saw this with [org.clojure/clojurescript "0.0-2371"]

The last line of the following, throws the error, when compiled with :output-wrapper

goog.provide("goog.string");
goog.provide("goog.string.Unicode");
goog.define("goog.string.DETECT_DOUBLE_ESCAPING", false);
goog.string.Unicode = {NBSP:"\u00a0"};

Notice how if fails right after providing goog.string, so it might be a bug with google closure.

Comment by Immo Heikkinen [ 14/Oct/14 12:10 AM ]

I noticed that if you move

var goog = goog || {};

outside the wrapper function, the problem goes away. Looks like tt's some kind of JavaScript scope issue.

Comment by David Nolen [ 14/Oct/14 5:22 AM ]

I suspect that the whitespace and output wrapper combination is not compatible. I'm inclined to close this one as it's a Closure limitation not a ClojureScript one. I would be open to an error when these two options are combined.

Comment by Immo Heikkinen [ 27/Oct/14 6:06 AM ]

Patch for throwing error when :output-wrapper and :optimizations :whitespace are combined

Comment by David Nolen [ 31/Oct/14 2:51 AM ]

Before proceeding this ticket needs more information. I would like confirmation that it is actually a Closure limitation first. The easiest way to confirm is to use Google Closure directly on some simple JavaScript.





[CLJS-878] Missing preamble file causes unhelpful stack trace Created: 30/Oct/14  Updated: 31/Oct/14  Resolved: 31/Oct/14

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

Type: Defect Priority: Trivial
Reporter: Darrick Wiebe Assignee: Unassigned
Resolution: Duplicate Votes: 0
Labels: cljs, errormsgs


 Description   

I think it would be helpful to throw an exception with the missing file path rather than the current

java.lang.IllegalArgumentException: No implementation of method: :make-reader of protocol: #'clojure.java.io/IOFactory found for class: nil
.

Right now you need to work your way down about 10 lines into the stacktrace to find

cljs.closure$preamble_from_paths$fn__3097.invoke(closure.clj:526)
which is the only hint as to what has happened.



 Comments   
Comment by David Nolen [ 31/Oct/14 2:42 AM ]

This is a simple enhancement, a patch is welcome. Thanks.

Comment by David Nolen [ 31/Oct/14 2:44 AM ]

Actually this already appears fixed in master https://github.com/clojure/clojurescript/commit/9fd6bf5bd55421c3d5becacc5230ed661d6fb3c3

Comment by David Nolen [ 31/Oct/14 2:45 AM ]

See CLJS-869





[CLJS-877] to-string of js/Date objects is inconsistent Created: 30/Oct/14  Updated: 30/Oct/14

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

Type: Defect Priority: Major
Reporter: Luke VanderHart Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None
Environment:

0.0-2371, Chrome



 Description   

When converting a js/Date object to a string, different things happen depending on whether the object is nested in a data structure or not.

For example:

(def date (js/Date.))
(str date) ;; => "Thu Oct 30 2014 16:49:50 GMT-0400 (EDT)"
(str [date]) ;; => "[#inst \"2014-10-30T20:49:50.999-00:00\"]"





[CLJS-876] merged sourcemap doesn't account for output-wrapper Created: 25/Oct/14  Updated: 27/Oct/14

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

Type: Defect Priority: Major
Reporter: J. David Lowe Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None

Attachments: Text File 876.alternative.patch     Text File 876.patch    

 Description   

When merging sourcemaps, the cljs.closure/optimize code correctly accounts for "preamble" lines that get prepended to closure compiler output. However, it fails to account for the extra lines added when :output-wrapper is true. As a result, the merged sourcemap is incorrect whenever :output-wrapper is true. (And for extra action-at-a-distance: :output-wrapper is implicitly set to true by lein-cljsbuild whenever :optimizations is :advanced.)

Too tired to make a patch tonight, but I'll go through the CA process and send a patch in a day or two, if nobody gets to it before then.



 Comments   
Comment by J. David Lowe [ 27/Oct/14 2:42 PM ]

Here are two patches, which both fix the problem.

876.patch is tiny, but I think it's probably a step in the wrong direction, because all future output prefixes will need the same fix applied to them.

876.alternative.patch is a deeper change, but I think is a better solution: it makes it much more apparent that all output prefixes should be added via make-preamble. The downside (IMO) is that the output-wrapper prefix and suffix are now separated a bit in the code.





[CLJS-710] port clojure.pprint Created: 02/Dec/13  Updated: 27/Oct/14

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

Type: Task Priority: Major
Reporter: David Nolen Assignee: Unassigned
Resolution: Unresolved Votes: 1
Labels: None


 Comments   
Comment by Shaun LeBron [ 15/Aug/14 1:29 PM ]

working on this here: https://github.com/shaunlebron/cljs-pprint

Comment by Shaun LeBron [ 05/Oct/14 12:40 PM ]

I'm ceasing development on the port.

fipp should be ported in place of pprint in cljs. pprint is slow and complex, whereas fipp is fast, simple, and very customizable.

Brandon Bloom is awaiting a go-ahead from the cljs community on whether the fipp port should be completed:
https://github.com/brandonbloom/fipp/issues/7

Comment by David Nolen [ 05/Oct/14 12:55 PM ]

fipp only covers a very small (though important part) of clojure.pprint's functionality. I think it's fine that fipp is a standalone library that user's can adopt in their own projects if they desire. This doesn't change the desire for a full port of clojure.pprint.

Comment by Shaun LeBron [ 27/Oct/14 1:13 PM ]

k, resuming.





[CLJS-851] simplify :none script inclusion if :main supplied Created: 05/Sep/14  Updated: 24/Oct/14

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

Type: Defect Priority: Major
Reporter: David Nolen Assignee: Andrew Rosa
Resolution: Unresolved Votes: 0
Labels: None


 Description   

If :main namespace supplied - under :none optimization settings :output-to file should goog.require it. This would also allow script inclusion to be unified regardless of the level of optimization supplied, i.e.:

<script type="text/javascript" src="foo.js"></script>

Instead of

<script type="text/javascript" src="out/goog/base.js"></script>
<script type="text/javascript" src="foo.js"></script>
<script type="text/javascript">goog.require("foo.core");</script>


 Comments   
Comment by David Nolen [ 05/Sep/14 5:10 PM ]

This does mean concatenating the contents of goog.base into the deps file.

Comment by Thomas Heller [ 06/Sep/14 8:31 AM ]

Just a quick Note: Not sure what your plan is for :main but it in my experience it is important to make it a collection. A larger app quickly gains multiple entry points.

Comment by David Nolen [ 06/Sep/14 9:38 AM ]

I don't see the point of making it a collection, users can simply load other entry points themselves via the "main" namespace.

Comment by Andrew Rosa [ 14/Oct/14 5:28 PM ]

Is someone already working on this issue? I will familiarize with the compiler code to try tackle this feature.

Comment by Andrew Rosa [ 15/Oct/14 10:17 PM ]

@dnolen I made the changes to compiler for auto require an specified :main namespace (but still optional). After that I started to experiment adding the goog.base code directly into the generated js, just to see what happens.

What I found is that we will need to disable the Closure default deps.js file. So we need to copy the goog.base code and after that include all dependencies, including the Closure ones. This work is need to avoid two issues:

  • We need to explicit set the base path for loading, since the auto-discovery method used by Closure searches for a <script str="base.js">.
  • By default, Closure automatically includes a <script> containing the goog/deps.js file to include itself. This causes race conditions during our require('main-ns'), since only half of our dependencies was added to dependency graph. Explicitly requiring everything will avoid this bug.

I could change the compiler to accommodate all this changes, but first like to communicate you the deep effects on our current behavior. What do you think?

Comment by Thomas Heller [ 16/Oct/14 2:24 AM ]

I have implemented this feature in shadow-build [1] and use the following strategy:

Assuming our output file is named "app.js" for an optimized build, it will look basically like this when compiling with :none.

1) imul.js fix
2) closure global settings

I found that these are required

var CLOSURE_NO_DEPS = true;
var CLOSURE_BASE_PATH = '/assets/js/src/';

The CLOSURE_BASE_PATH is required for the include scripts to find the javascripts for each namespace.

3) After the settings the goog/base.js is emitted into the file.
4) After that the contents of dependency settings are emitted (basically goog/deps.js) plus all of our namespaces. They look something like this

goog.addDependency("clojure/set.js",['clojure.set'],['cljs.core']);

When closure attempts to load a given namespace it will simply emit a <script src="CLOSURE_BASE_PATH + path/to/ns.js">, I found that using an absolute path is the best way to ensure that files are loaded correctly since the "path detection" feature of the goog/base.js file fails since we do not include a goog/base.js

5) Then finally you can just add the

goog.require('main-ns');

Hope that helps.

[1] https://github.com/thheller/shadow-build

Comment by Andrew Rosa [ 16/Oct/14 7:35 AM ]

Thank you Thomas for your very nice writeup.

In my tests I found that we could leave CLOSURE_BASE_PATH alone IF we made everything relative to our output file. Do you think forcing it to some path is more reliable?

Comment by Thomas Heller [ 16/Oct/14 1:08 PM ]

Yes definitly. The location of the javascript files usually is "fixed" once the app is configured. Either you put it locally at something like "/js" or something like a CDN, so you configure it once and forget about it. But thats my own personal opinion, I have no idea how other people are handling it. I had some problems with relative paths but can't remember exactly what they were only that CLOSURE_BASE_PATH fixed it.

Comment by Andrew Rosa [ 24/Oct/14 4:52 PM ]

Just to give an update here: I already have a patch for this issue, but depends on the resolution of CLJS-874.





Generated at Fri Oct 31 12:12:18 CDT 2014 using JIRA 4.4#649-r158309.