<< Back to previous view

[CLJS-2279] Infer `:module-type ` for provided `node_modules` Created: 27/Jul/17  Updated: 28/Jul/17

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

Type: Enhancement Priority: Major
Reporter: António Nuno Monteiro Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: js-modules

Approval: Vetted

 Description   

When crawling a `node_modules` installation, we currently hardcode every provided `:module-type` to `:commonjs`. However, certain packages are starting to provide ES6 modules and we could do a better job at figuring out which files provide CommonJS vs. ES6 modules.

This might be slightly hard to do and we should throw around some ideas on how to accomplish it.



 Comments   
Comment by Thomas Heller [ 28/Jul/17 3:01 AM ]

Many packages also ship CommonJS AND ES6 together and use the "package.json" "module" key for the ES6 code and "main" for normal CommonJS.

See:
https://github.com/rollup/rollup/wiki/pkg.module
https://github.com/nodejs/node-eps/pull/60

However many packages also do weird mixed stuff where the entry is ES6 but then uses require in the imported files (eg. material-ui/index.es.js). So it is not safe to assume that once you enter ES6 it will stay ES6 although the spec suggests otherwise.

FWIW in my own experiments I did not run into any issues ALWAYS setting :language-in to :ecmascript6, (.setProcessCommonJSModules true) and (.setTransformAMDToCJSModules true). Closure seems to be smart enough to figure things out on its own? Their JsFileParser is also able to detect ES6 files (by checking if a file contains import/export) without actually parsing.





[CLJS-2278] JavaScript object literals are printed wth keys that cannot be read Created: 27/Jul/17  Updated: 27/Jul/17

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

Type: Defect Priority: Major
Reporter: Mike Fikes Assignee: David Nolen
Resolution: Unresolved Votes: 1
Labels: print

Attachments: Text File CLJS-2278.patch    
Patch: Code and Test
Approval: Vetted

 Description   

If you evaluate #js {"foo bar" 2}, it prints as #js {:foo bar 2}.

I'd suggest that, for JavaScript keys that cannot be represented as unqualified keywords, string notation would be used.

Example:

#js {"alpha" 1 "beta gamma" 2 "delta/epsilon" 3}

could print as

#js {:alpha 1 "beta gamma" 2 "delta/epsilon" 3}

Note that the ability to avoid namespace maps was added with CLJS-2190, which is in master.



 Comments   
Comment by Mike Fikes [ 27/Jul/17 9:13 PM ]

The attached patch conditionally converts the JavaScript object key from a string to a keyword iff it is a string that can be converted to a legal unqualified keyword.

By "legal": Abides with the first bullet under Symbols https://clojure.org/reference/reader#_symbols, doesn't allow / (because the resulting keyword would be qualified) and doesn't allow . (because the description of keywords below disallows .).

The only bit that is not supported is the last bullet under Symbols: "A symbol can contain one or more non-repeating :'s." (but only inside the string, not at the beginning nor end). I'm presuming that this code is not in an overly performance critical code path, and that a regex approach is sufficiently performant, but trying to match this last aspect might make for a slow regex, compared to the simple one that simply allows zero or more characters from a given character class.





[CLJS-2277] Document return value of js-delete Created: 27/Jul/17  Updated: 27/Jul/17

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

Type: Enhancement Priority: Trivial
Reporter: Mike Fikes Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: docstring


 Description   

Document that js-delete returns true upon success, false otherwise.






[CLJS-2276] Self-host: Need test.check dep for CLJS-2275 Created: 27/Jul/17  Updated: 27/Jul/17  Resolved: 27/Jul/17

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

Type: Defect Priority: Minor
Reporter: Mike Fikes Assignee: David Nolen
Resolution: Completed Votes: 0
Labels: bootstrap

Attachments: Text File CLJS-2276.patch    
Patch: Code and Test
Approval: Accepted

 Description   

The change in CLJS-2275 requires that we be able to load test.check in self host.



 Comments   
Comment by Mike Fikes [ 27/Jul/17 2:54 PM ]

The attached patch re-adds the support in script/test-self-parity.

Comment by Mike Fikes [ 27/Jul/17 3:11 PM ]

For historical context, this patch re-adds the stuff that was in CLJS-2082, which we had later reverted owing to the fact that it added a dep on org.clojure/test.check 0.10.0-alpha1, which introduced a bug TCHECK-131 which would end up in the shipping cljs.jar. That bug has now been fixed, with the fixe released in org.clojure/test.check 0.10.0-alpha2. We are actually now on that version of test.check owing to CLJS-2273, so all this patch really does is add the infrastructural stuff to script/test-self-parity so that it can load test.check, along with the sample generative tests. This is sufficient to allow test.check to be loaded for CLJS-2275.

Comment by David Nolen [ 27/Jul/17 8:15 PM ]

fixed https://github.com/clojure/clojurescript/commit/190fa6489c9578ec7dba8235e5c905ae32ff7fda





[CLJS-2275] cljs.spec.alpha/fdef resolves eagerly Created: 27/Jul/17  Updated: 27/Jul/17  Resolved: 27/Jul/17

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

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

Approval: Accepted

 Description   
(s/fdef foo.bar/kw->str
  :args (s/cat :kw keyword?)
  :ret  string?)

Triggers an error, in clojure.spec.alpha this works



 Comments   
Comment by David Nolen [ 27/Jul/17 1:58 PM ]

fixed https://github.com/clojure/clojurescript/commit/18761390b13d0dc4d80c0925c75d3eab49279ba3





[CLJS-2274] Update CI script to install deps Created: 26/Jul/17  Updated: 27/Jul/17  Resolved: 27/Jul/17

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

Type: Task Priority: Critical
Reporter: António Nuno Monteiro Assignee: David Nolen
Resolution: Completed Votes: 0
Labels: None

Attachments: Text File CLJS-2274.patch    

 Description   

Missed this in CLJS-2272



 Comments   
Comment by David Nolen [ 27/Jul/17 6:36 AM ]

fixed https://github.com/clojure/clojurescript/commit/5ebf0806701a48cf9ca577af8ea76287b2975b32





[CLJS-2273] Bump tools.reader to 1.0.3 and development dependencies Created: 25/Jul/17  Updated: 26/Jul/17  Resolved: 26/Jul/17

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

Type: Task Priority: Critical
Reporter: António Nuno Monteiro Assignee: David Nolen
Resolution: Completed Votes: 0
Labels: None

Attachments: Text File CLJS-2273.patch    
Approval: Accepted

 Comments   
Comment by David Nolen [ 26/Jul/17 4:20 AM ]

fixed https://github.com/clojure/clojurescript/commit/6c2d17e3e3bfc725c52f6cadb7a808693f07c8d1





[CLJS-2272] Tests that depended on default install deps behavior failing Created: 25/Jul/17  Updated: 25/Jul/17  Resolved: 25/Jul/17

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

Type: Defect Priority: Blocker
Reporter: António Nuno Monteiro Assignee: David Nolen
Resolution: Completed Votes: 0
Labels: js-modules, test

Attachments: Text File CLJS-2272.patch    
Patch: Code and Test

 Comments   
Comment by David Nolen [ 25/Jul/17 3:18 PM ]

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





[CLJS-2271] native-satisfies? using aget Created: 24/Jul/17  Updated: 24/Jul/17  Resolved: 24/Jul/17

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

Type: Defect Priority: Minor
Reporter: Mike Fikes Assignee: Mike Fikes
Resolution: Not Reproducible Votes: 0
Labels: None


 Description   

native-satisfies? is using aget and should instead be using either gobj/get or unchecked-get.



 Comments   
Comment by Mike Fikes [ 24/Jul/17 9:33 AM ]

Already fixed with https://github.com/clojure/clojurescript/commit/84a2128dab9f52e67ee227a66be4f849d83de0a3





[CLJS-2270] Support AOT compilation of macro namespaces (bootstrap) Created: 24/Jul/17  Updated: 24/Jul/17

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

Type: Enhancement Priority: Major
Reporter: David Nolen Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: bootstrap

Approval: Vetted




[CLJS-2269] Warn on top level code split loads Created: 24/Jul/17  Updated: 26/Jul/17  Resolved: 26/Jul/17

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

Type: Enhancement Priority: Critical
Reporter: David Nolen Assignee: Unassigned
Resolution: Completed Votes: 0
Labels: modules

Approval: Vetted

 Description   

See CLJS-2264 & and CLJS-2265. We should warn on top-level module loading and provide some basic guidance. Docs should make expectations clear.



 Comments   
Comment by David Nolen [ 26/Jul/17 4:07 AM ]

fixed https://github.com/clojure/clojurescript/commit/364c2ff66bf7e7ff11a28d9366f3f98a82ffdbcb





[CLJS-2268] clojure.string/escape in ClojureScript (unlike in Clojure) assumes cmap is a map Created: 23/Jul/17  Updated: 23/Jul/17

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

Type: Defect Priority: Minor
Reporter: Max Kreminski Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: cljs


 Description   

The ClojureScript implementation of the clojure.string/escape function assumes that the cmap parameter will always be a map. This makes it different from (and specifically less general than) the Clojure implementation of the same function, which permits cmap to be anything callable.

Here's the relevant lines of the clojure.string/escape implementations in Clojure and ClojureScript. The ClojureScript implementation calls get on cmap, while the Clojure implementation invokes cmap directly.

Here's an example that works on Clojure, but doesn't work on ClojureScript, because it passes a function to clojure.string/escape instead of a map:

(defn regex-escape
  "Escapes regex special chars in the string `s`."
  [s]
  (let [special? #{\- \[ \] \{ \} \( \) \* \+ \? \. \\ \^ \$ \|}]
    (clojure.string/escape s #(when (special? %) (str \\ %)))))

Ideally, this discrepancy would be fixed by changing the ClojureScript implementation of clojure.string/escape to follow the Clojure one. This would also match the behavior described in the function's docstring, which is the same on both platforms.






[CLJS-2267] Allow ^:const inlined vars to affect if emission Created: 21/Jul/17  Updated: 24/Jul/17  Resolved: 24/Jul/17

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

Type: Enhancement Priority: Major
Reporter: Mike Fikes Assignee: David Nolen
Resolution: Completed Votes: 0
Labels: core

Attachments: Text File CLJS-2267.patch    
Patch: Code and Test
Approval: Accepted

 Description   

If an if test is a truthy or falsey constant, this fact is used to emit either the then or else branch, while also avoiding Closure warnings about unreachable code.

With the new ^:const Var inlining feature, we can allow a modest broadening of scope to also allow for truthy or falsey const expressions, perhaps slightly improving performance of affected code that is not run through Closure, and also avoiding new unreachable code warnings that can otherwise now crop up with the new JavaScript that results from ^:const Var inlining.



 Comments   
Comment by David Nolen [ 24/Jul/17 12:59 AM ]

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





[CLJS-2266] Self-host: Cannot require clojure.x where clojure.x has no macros namespace Created: 21/Jul/17  Updated: 21/Jul/17

Status: Open
Project: ClojureScript
Component/s: None
Affects Version/s: 1.9.655, 1.9.660, 1.9.671
Fix Version/s: None

Type: Defect Priority: Minor
Reporter: Viktor Magyari Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: bootstrap


 Description   

Repro:

(require 'cljs.js)

(cljs.js/eval-str (cljs.js/empty-state)
                  "(require 'clojure.x)"
                  nil
                  {:eval cljs.js/js-eval
                   :load (fn [{:keys [name macros]} cb]
                           (cb (when (and (= name 'cljs.x)
                                          (not macros))
                                 {:lang   :clj
                                  :source "(ns cljs.x)"})))}
                  println)

Expected result:

{:ns cljs.user, :value nil}

Actual result:

{:error #error {:message No such macros namespace: cljs.x, could not locate cljs/x.clj or cljs/x.cljc, :data {:tag :cljs/analysis-error}}}

Git bisect indicates CLJS-2069:

d4db18970c8eec587b2c9e022034983e29eb8e81 is the first bad commit
commit d4db18970c8eec587b2c9e022034983e29eb8e81
Author: António Nuno Monteiro <anmonteiro@gmail.com>
Date:   Sat Jun 3 15:10:40 2017 -0700

    CLJS-2069: Self-host: automatic `clojure` -> `cljs` aliasing doesn't work when loading macro namespaces

:040000 040000 31b0ec3b5346c8959e48487449576114b9d9a2ea 59d730263393923274257ea1eadaf4c2828c4199 M      src


 Comments   
Comment by Mike Fikes [ 21/Jul/17 1:20 PM ]

A concrete example of this is the inability to require cljs.tools.reader by indicating clojure.tools.reader.





Generated at Fri Jul 28 03:50:18 CDT 2017 using JIRA 4.4#649-r158309.