resolve-in-jar-dep is a wee bit brittle

[RyanMcG]

I received the following error message when running lein deps.

java.lang.ClassCastException: clojure.lang.PersistentVector cannot be cast to clojure.lang.Named
 at clojure.core$name.invoke (core.clj:1518)
    leiningen.npm.deps$resolve_in_jar_dep.invoke (deps.clj:51)
    clojure.lang.AFn.applyToHelper (AFn.java:160)
    clojure.lang.AFn.applyTo (AFn.java:144)
    clojure.core$apply.invoke (core.clj:628)
    clojure.core$partial$fn__4230.doInvoke (core.clj:2470)
    clojure.lang.RestFn.invoke (RestFn.java:408)
    clojure.core$keep$fn__6402.invoke (core.clj:6713)
    clojure.lang.LazySeq.sval (LazySeq.java:40)
    clojure.lang.LazySeq.seq (LazySeq.java:56)
    clojure.lang.RT.seq (RT.java:484)
    clojure.core$seq.invoke (core.clj:133)
    clojure.core.protocols$seq_reduce.invoke (protocols.clj:26)
    clojure.core.protocols/fn (protocols.clj:53)
    clojure.core.protocols$fn__6031$G__6026__6044.invoke (protocols.clj:13)
    clojure.core$reduce.invoke (core.clj:6287)
    leiningen.npm.deps$resolve_in_jar_deps.invoke (deps.clj:63)
    leiningen.npm.deps$resolve_node_deps.invoke (deps.clj:82)

(let [jar-project-entry (.getEntry jar-file "project.clj")
      jar-project-src (when jar-project-entry
                        (read-string                                        ; 1
                          (slurp                                            ;
                            (.getInputStream jar-file jar-project-entry)))) ;
      jar-project-map (when jar-project-src
                        (->> jar-project-src (drop 3) (apply hash-map)))
      jar-project-name (when jar-project-src
                         (name (second jar-project-src))) ; 2
      ...

1. This may not actually get a form with defproject being invoked in it. I've done this before:

(read-string (str \( a-string-of-clojure-code \)))

Of course, this will change result in a list of forms so some searching needs to take place to find the defproject usage. 2. Here we assume, we've found a totally normal defproject, we call second and name to get a string representing the name of the project. Unfortunately, for a project.clj like reply's this fails and breaks the whole thing.

(let [dev-deps '[[speclj "2.7.2"]
             [classlojure "0.6.6"]]]

(defproject reply "0.3.5-SNAPSHOT" ...

I am not convinced that pulling node dependencies of maven dependencies is the right thing to do. I'd like to get a better understanding of why this is done. Regardless, I think and ideal solution would put the dependencies in the pom and interpret them from there instead of trying to read project.clj.

A good, short term solution would be to provide better error handling so that when a project.clj is improperly interpreted, it is simply ignored. Another, would be to surface an option which disables gathering node-dependencies on sub projects. I think there are cases where it is undesired.

I would be happy to submit a pull-request once we know what might work best!

[RyanMcG]

I'll post a PR with a fix for the assumptions made about the forms in project.clj soon, it's already done, it's just on my personal computer.

[RyanMcG]

ping

[radhikalism]

As a sidenote, leiningen 2.4 begins to deal with a similar problem, which supposes a general solution for project map processing that might apply to lein-npm.

The newly introduced 'release' task involves reflecting on the contents of project.clj, and Phil H notes that a general 'change' task for reading and rewriting project maps might eventually be useful. http://librelist.com/browser//leiningen/2014/5/1/release-task/

If that ever materializes as a robust API in leiningen, it could be reused by plugins that extract or inject values like lein-npm does. So I don't know if transmitting data via the pom file is the best course of action.

+1 for this PR.

[RyanMcG]

@arbscht I may be mis-interpretting you or the linked post to the leiningen mailinglist. But this seems to be referring to leiningen tasks which rewrite project.clj. Though I think this could be useful for leningen or lein-npm I am not sure it is solving the problem I am bringing up here.

Our issue is with reliable reading a project definition of a dependency. This is because (defproject ...) is code. Through the beauty of LISP we are able to read this trivially, but not necessarily perfectly (we've the PR I've already posted is a good example of how making assumptions about the structure of project.clj can bite us). I do not think we have to wait for some yet-to-be-implemented change task. Leiningen already provides functions for reading project.clj files. I feel like we should probably use one, if we can't read pom files directly. (Maybe this will be my next PR)

I was originally under the impression that just reading pom files would be a significant performance improvement, but I don't think that is the case anymore.

[radhikalism]

@RyanMcG You're right, of course. I just mean that working with project.clj and not pom files is likely to be a better long-term direction, as this is where all the action happens in a leiningen context (including unrelated rewrites, eventually). The pom file should probably remain an implementation detail for leiningen itself to manage, even for lein plugins, rather than making it a leaky abstraction.

Using leiningen's read for project files sounds like an excellent idea — if it is now in a workable state. I forget now why it wasn't sufficient when I last looked at it, but there have been many changes to that namespace since then.

[RyanMcG]

@arbscht Alright, I agree. We should avoid relying on pom and go the read route, if it works.

@bodil thoughts on #15?

[bodil]

I think this project needs a maintainer (I'm not doing Clojure anymore, so not able to support it). Anyone interested?

[RyanMcG]

I am. Full disclosure, I might ask for some non-clojure-specific library maintenance help though.

I'd be happy to co-maintain with someone else too. We could have a cljs-npm organization or something.

[bodil]

OK, how about I just transfer it to you, then you're free to create an organisation if more people want to pitch in?

[radhikalism]

Excellent, thanks @RyanMcG!

(I'm very much interested in this project but cannot help as a maintainer at this time. Happy to contribute and support otherwise.)

[RyanMcG]

@bodil transfer away!

[bodil]

Transfer in progress!

[bodil]

Oh, actually, @RyanMcG, you'll need to delete your fork first, getting "ryanmcg/lein-npm already exists" sort of errors.

[RyanMcG]

@bodil done!

Well, I renamed my current fork at least, but that should fix it.

[bodil]

Apparently not.

"RyanMcG already has a repository in the bodil/lein-npm network"

[RyanMcG]

OK, deleted. Take 3. Sorry about that. https://github.com/RyanMcG/lein-npm 404s now instead of redirecting

[bodil]

Seems to have worked. Enjoy the repo. :)

[RyanMcG]

Ha, thanks.

[RyanMcG]

Well, I am going to merge in #15 then.

[olivergeorge]

I think this bug renders lein-npm incompatible with chestnut.

[RyanMcG]

@olivergeorge can you elaborate? Have you tried current master?

[olivergeorge]

Hi Ryan

Just tried "0.5.0-SNAPSHOT" and that works.

Test was:

• lein new chestnut npmtest

• add npm plugin and node-dependenies to project.clj based on readme

• lein npm install
• See error with 0.4.0

• update npm plugin reference to 0.5.0-SNAPSHOT

• lein npm install
• See install work

cheers, Oliver

[RyanMcG]

OK, so it sounds like I should cut a release. I'll try to do that in the next couple of days. On Dec 26, 2014 4:04 PM, "Oliver George" [email protected] wrote:

Hi Ryan

Just tried "0.5.0-SNAPSHOT" and that works.

Test was:

• lein new chestnut npmtest

• add npm plugin and node-dependenies to project.clj based on readme

• lein npm install
• See error with 0.4.0

• update npm plugin reference to 0.5.0-SNAPSHOT

• lein npm install
• See install work

cheers, Oliver

— Reply to this email directly or view it on GitHub https://github.com/RyanMcG/lein-npm/issues/14#issuecomment-68158054.

[olivergeorge]

Brilliant, thanks.

On 27 December 2014 at 08:39, Ryan McGowan [email protected] wrote:

OK, so it sounds like I should cut a release. I'll try to do that in the next couple of days. On Dec 26, 2014 4:04 PM, "Oliver George" [email protected] wrote:

Hi Ryan

Just tried "0.5.0-SNAPSHOT" and that works.

Test was:

• lein new chestnut npmtest

• add npm plugin and node-dependenies to project.clj based on readme

• lein npm install
• See error with 0.4.0

• update npm plugin reference to 0.5.0-SNAPSHOT

• lein npm install
• See install work

cheers, Oliver

— Reply to this email directly or view it on GitHub https://github.com/RyanMcG/lein-npm/issues/14#issuecomment-68158054.

— Reply to this email directly or view it on GitHub https://github.com/RyanMcG/lein-npm/issues/14#issuecomment-68159297.

[RyanMcG]

@bodil I'm trying to cut a release but I am getting an "Access denied" error.

Can you add me to the lein-npm group on clojars? https://clojars.org/groups/lein-npm

Thanks :smile:

[bodil]

Oops, sorry, was sure I already had.

[RyanMcG]

@bodil that did it! @olivergeorge release cut!

[olivergeorge]

Thanks Ryan.

Seems like a interesting time for CLJS with JS dependencies.

What are you making of the JS build conversations happening at the moment? David's raised a ticket about tweaking :foreign-libs and boot-cljsjs was announced recently. Sounds like Google Closure is working towards AMD support too. Related discussions happening on the mailing list.

On 29 December 2014 at 07:58, Ryan McGowan [email protected] wrote:

@bodil https://github.com/bodil that did it! @olivergeorge https://github.com/olivergeorge release cut!

— Reply to this email directly or view it on GitHub https://github.com/RyanMcG/lein-npm/issues/14#issuecomment-68219247.

[RyanMcG]

No problem.

Honestly, I haven't looked into much. Any chance you can link me that ticket?