<< Back to previous view

[CLJS-2018] User supplied externs not loaded with user specified compiler state Created: 25/Apr/17  Updated: 26/Apr/17

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

Type: Defect Priority: Major
Reporter: Jonathan Henry Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None

Attachments: Text File CLJS-2018.patch    

 Description   

User supplied externs for use with warn-on-infer are only loaded in the 2-arity versions of cljs.closure/build, cljs.build.api/build and cljs.build.api/watch when compiler is nil.

Note: This only affects the warnings that are generated by the analyzer with warn-on-infer; the externs are correctly passed to gclosure.



 Comments   
Comment by Jonathan Henry [ 25/Apr/17 7:34 PM ]

This patch moves the loading of externs from cljs.env/default-compiler-env to cljs.closure/build.

Comment by Jonathan Henry [ 26/Apr/17 12:20 PM ]

Ignore this patch, I just realized this makes it so the built-in externs are no longer loaded for the compiler and analyzer API.





[CLJS-2013] Add MapEntry type Created: 19/Apr/17  Updated: 26/Apr/17

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

Type: Enhancement Priority: Minor
Reporter: Thomas Mulvaney Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None

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

 Description   

It would be nice to have a MapEntry type in ClojureScript.

Since the introduction of the map-entry? predicate in Clojure 1.8 it has been trivial to determine if an object represents an entry in a map. Being able to distinguish a map entry from other data types is really handy when walking data amongst other things. Currently, ClojureScript uses PersistentVectors to represent entries from PersistentArrayMaps and PersistentHashMaps. PersistentTreeMaps use their own nodes to represent entries and as such would be unaffected.

Besides making it possible to use the the map-entry? predicate across both CLJ and CLJS code, there may space/performance benefits to having a specialised type.



 Comments   
Comment by Thomas Mulvaney [ 26/Apr/17 11:05 AM ]

I've attached a patch which introduces the MapEntry type. It's there but not hooked up - PAMs and PHMs don't emit them when you `seq` or `iterate` over them yet.

There is a set of tests which check the protocols all map entries should implement: IVector, IAssociative, ISeq etc. The exact same set of tests are run across PersistentVector, RedNode, BlackNode and the new MapEntry type to ensure they behave identically.

A few bugs were caught in doing so:

  • -nth on RedNodes and BlackNodes with an out of bounds index did not throw an exception.
  • -invoke with an out of bounds index had the same problem
  • -find behaviour on Red and Black nodes did not behave as expected. See CLJS-2014
  • PersistentVectors, RedNodes and BlackNodes all implement IAssociative but missed the -contains-key? method.

The listed bugs were fixed in the patch.





[CLJS-2017] Upgrade Closure Compiler to latest April 2017 release Created: 25/Apr/17  Updated: 25/Apr/17

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

Type: Enhancement Priority: Minor
Reporter: António Nuno Monteiro Assignee: David Nolen
Resolution: Unresolved Votes: 0
Labels: closure

Attachments: Text File CLJS-2017.patch    
Patch: Code

 Description   

The Closure Compiler team has released a new version that includes more enhancements to JS module processing






[CLJS-2016] Support inheritance annotations in externs Created: 25/Apr/17  Updated: 25/Apr/17

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

Type: Enhancement Priority: Major
Reporter: Jonathan Henry Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None


 Description   

Closure externs may contain @extends annotations to specify base classes. The base classes' attributes should be propagated to subclasses in `:cljs.analyzer/externs`.






[CLJS-2015] Self-host: `defmacro` should return the Var Created: 24/Apr/17  Updated: 24/Apr/17

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

Type: Defect Priority: Minor
Reporter: António Nuno Monteiro Assignee: David Nolen
Resolution: Unresolved Votes: 0
Labels: bootstrap

Attachments: Text File CLJS-2015.patch    
Patch: Code




[CLJS-1444] Node.js shim requires `node` invocation to be in the same directory as shim script Created: 10/Sep/15  Updated: 24/Apr/17

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

Type: Defect Priority: Minor
Reporter: Martin Klepsch Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None

Attachments: Text File 0001-In-bootstrapping-code-use-__dirname-to-calculate-pat.patch    

 Description   

Don't have time to provide a proper repro now but the basic issue can be illustrated by this:

~/c/boot-cljs-example (master=) node target/main.js
module.js:338
    throw err;
          ^
Error: Cannot find module '/Users/martin/code/boot-cljs-example/out/goog/bootstrap/nodejs.js'
    at Function.Module._resolveFilename (module.js:336:15)
    at Function.Module._load (module.js:278:25)
    at Module.require (module.js:365:17)
    at require (module.js:384:17)
    at Object.<anonymous> (/Users/martin/code/boot-cljs-example/target/main.js:6:1)
    at Module._compile (module.js:460:26)
    at Object.Module._extensions..js (module.js:478:10)
    at Module.load (module.js:355:32)
    at Function.Module._load (module.js:310:12)
    at Function.Module.runMain (module.js:501:10)
~/c/boot-cljs-example (master=) cd target/
~/c/b/target (master=) node main.js
Starting...

This is compiled with boot because that was what I had at hand right now. The compiled shim looks like this:

var path = require("path");
try {
    require("source-map-support").install();
} catch(err) {
}
require(path.join(path.resolve("."),"out","goog","bootstrap","nodejs.js"));
require(path.join(path.resolve("."),"out","cljs_deps.js"));
goog.global.CLOSURE_UNCOMPILED_DEFINES = {"cljs.core._STAR_target_STAR_":"nodejs"};
goog.require("boot.cljs.main");
goog.require("cljs.nodejscli");

The problem here is that path.resolve(".") will return the directory the node command was invoked in and not the directory of the shim. (See the "Cannot find module..." error above)

A solution could be to use __dirname which always resolves to the directory of the current file. This might result in some breakage for existing setups.



 Comments   
Comment by Sebastian Bensusan [ 14/Oct/15 11:31 AM ]

I have a proposed solution but I fear that supporting "run from anywhere" adds essential complexity to the resulting code. My thought process:

1. Relative paths are desirable to produce "context free code." If the user chooses absolute paths, then that behavior is respected and there's nothing to handle (no "path algebra") .

2. When dealing with relative paths, the whole system needs to establish a "frame of reference", a root path. The ClojureScript compiler assumes the path from which it is compiling to be that frame of reference, which usually coincides with the top root of the project. Though arbitrary, it is the only choice that makes sense.

3. The frame of reference is not explicit anywhere in the code, since it is defined as ".". If it were explicit, it would reveal context, as in "/home/some-user/their-folder/this-project/".

4. When we approach the code from another reference point (executing the script from another directory), we first need to find the original compiler path (reference point,) and then resolve all paths from there. The compiler uses `cljs.closure/path-relative-to` for this purpose.

Path algebra:
compiler-path = __dirname - output-to

Node.js

var compiler-path = __dirname.replace(output-to, "")
path.resolve (compiler-path, output-dir, "goog", "base.js")
path.resolve (compiler-path, output-dir, "cljs_deps.js")

which assumes that if output-to was given relatively, then output-dir is also relative. If they are not in sync, more work needs to be done to keep them that way.

It's not up to me to decide if the extra complexity is worth the use-case. I actually hope there is a simpler solution to solve this that I'm not seeing.

Comment by Karol Majta [ 14/Mar/16 10:43 AM ]

I find this behavior really weird and would opt for switching to __dirname. I am also not sure i fully understand consequences of such switch (I have little cljs experience, speaking more from the perspective of nodejs user). My point is: current behavior renders clojurescript hard to use for commandline and desktop applications (namely electron).

For command line and desktop applications assumptions about CWD cannot be made. For now i run my applications through a bootstrap script JS script:

process.chdir(__dirname);
require('./target/out');

I am lucky that my code does not have to use the real CWD, but it's a hack more than a real solution.

Speaking from nodejs perspective:

1. Using absolute paths is considered a bad practice anyway.
2. Nodejs programs that don't use external packages (don't depend on node_modules) can be run from any CWD
3. Nodejs programs that do depend on node_modules will break if run from a different directory than the one containing node_modules, but this is expected behavior.

Comment by J. Pablo Fernández [ 26/Jan/17 5:42 AM ]

I just run into this problem when trying to develop an Electron application. The way it's working right now is essentially unpackageable. I think it would be nice to have this behavior even as an option and I'm happy to work on a patch.

Comment by J. Pablo Fernández [ 26/Jan/17 5:57 AM ]

As far as I can see, this is the relevant code:

https://github.com/clojure/clojurescript/blob/cdaeff298e0f1d410aa5a7b6860232270d287084/src/main/clojure/cljs/closure.clj#L1410-L1411

Comment by J. Pablo Fernández [ 26/Jan/17 6:15 AM ]

A potential workaround seems to use :simple optimization.

Comment by Matt Lee [ 21/Apr/17 12:02 PM ]

I have also just hit this issue in an electron app that I'm building. @pupeno did you get anywhere with your offer to work on a patch? I too would be happy to look in to a patch for this. Although I think I'll need some pointers to get started.

Comment by Matt Lee [ 22/Apr/17 5:36 AM ]

From a quick experiment this morning it looks like replacing path.resolve(".") with __dirname at https://github.com/clojure/clojurescript/blob/aa5f001300e9aebd976cb180f5b7ccb37fcb6898/src/main/clojure/cljs/closure.clj#L1460-L1461 works for a couple of simple electron and node apps. By work I specifically mean that the error doesn't occur even when the app is started from a current working directory that is different to the compilation directory, i.e. the code is path independent.

I'll attach a patch for this once I've done some more complete testing. Before then any feedback on this approach would be appreciated.

Comment by Matt Lee [ 23/Apr/17 1:51 AM ]

Attaching patch for the suggested fix of using __dirname instead of "." in the generated script. I have tested this with nodejs 6.10.0 and electron 1.6.5.

Comment by Matt Lee [ 24/Apr/17 2:34 AM ]

Also see: CLJS-1990





[CLJS-1990] Clojurescript programs targeting nodejs should support global installation Created: 28/Mar/17  Updated: 24/Apr/17

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

Type: Enhancement Priority: Minor
Reporter: Greg Haskins Assignee: Unassigned
Resolution: Unresolved Votes: 0
Labels: None

Attachments: Text File 0001-CLJS-1990-Use-module-relative-__dirname-for-bootstra.patch    

 Description   

The top-level entry point in a :target :nodejs application uses $CWD relative paths to load the bootstrapping. See "path.resolve('.')" here: https://github.com/clojure/clojurescript/blob/296d0a69340e832b92ed742b3cd0304a06bea27f/src/main/clojure/cljs/closure.clj#L1460 for example.

This works fine for a local build, but is problematic when we try to globally install clojurescript (such as via 'npm install -g') because it requires the caller $CWD to be something that is likely to be unnatural (e.g. /usr/lib/node_modules/$pkg). Suggested fix is to replace "path.resolve('.')" with "__dirname".



 Comments   
Comment by Matt Lee [ 24/Apr/17 2:34 AM ]

Also see: CLJS-1444





Generated at Wed Apr 26 17:00:46 CDT 2017 using JIRA 4.4#649-r158309.