adzerk-oss / boot-cljs-repl

Boot task providing a REPL for ClojureScript development.
Eclipse Public License 1.0
72 stars 28 forks source link

missing dependencies #35

Open magomimmo opened 8 years ago

magomimmo commented 8 years ago

By switching from "0.2.0" release to "0.3.0" release I got the fallowing dependencies warning and error:

...
Writing boot_reload.cljs...
You are missing necessary dependencies for boot-cljs-repl.
Please add the following dependencies to your project:
[com.cemerick/piggieback "0.2.1" :scope "test"]
[weasel "0.7.0" :scope "test"]
[org.clojure/tools.nrepl "0.2.12" :scope "test"]
....
Adding :require adzerk.boot-reload to main.cljs.edn...
java.io.FileNotFoundException: Could not locate cemerick/piggieback__init.class or cemerick/piggieback.clj on classpath.
...
Elapsed time: 1.578 sec

It seems that the dependencies' transitivity is lost on the way. Obviously if I explicitly add the missing deps it works.

Deraen commented 8 years ago

Working as intended.

In 0.3.0 I made a decision to require direct dependencies to CLJS Repl libs because with current implementation it's impossible to reliably and easily add those automatically. Having boot-cljs-repl depend on the libs transitively is problematic because there is no guarantee that transitive dependencies from boot-cljs-repl are used. If several libs depend on e.g. piggieback, the nearest and first version is selected, which could be wrong version.

magomimmo commented 8 years ago

uhm, in this way we're breaking the most standard behaviour of any maven-based lib: https://maven.apache.org/guides/introduction/introduction-to-dependency-mechanism.html#Transitive_Dependencies

At least we should write an ATTENTION NOTE in the readme.md and explain the reason why as well. When you say with the current implementation you mean that you're working on a new implementation?

thanks

Deraen commented 8 years ago

I don't see how this breaks any behavior?

Boot tasks preferably shouldn't have any transitive dependencies and if they need to depend on something they should use pods. But the current implementation runs REPL on the "main" pod (where your app runs). Automatically adding dependencies to this pod (by calling set-env! :dependencies in task or transitive dependencies) can cause side-effects to the application so it should be avoided.

I've been talking about this with @micha and we think it should be eventually possible to run the REPL inside a pod.

Deraen commented 8 years ago

Okay, so maybe saying "doesn't break any behavior" is a bit wrong as this was breaking change. I have now fixed the change log to mention this and fixed a typo there.

I just think that standard behaviors of maven libs don't completely hold here because Boot tasks, like Leiningen plugins, are a bit of special case. Both should use their own mechanisms to add the dependencies in a way that doesn't have side-effects on application dependencies. (Leiningen tasks occasionally have to break this, which can cause problems like the plugin dependencies leaking to pom.xml on the jar).

I also added a mention and explanation on the readme.

magomimmo commented 8 years ago

Thanks!. Are those dependencies to be added always the same? In this case I would be more explicit in the note about them, otherwise the user has to first run the boot command to discover what are the deps to be added, add them e then rerun the boot command again.

Deraen commented 8 years ago

Yes. Though now that I think of, it's possible that we want to update them in the future.