yogthos / json-html

Provide EDN/JSON and get a DOM node with a human representation of the data
MIT License
162 stars 15 forks source link

json-html drags in cider dependency #15

Closed seancorfield closed 5 years ago

seancorfield commented 5 years ago

I just updated Selmer in our project at work and the latest version of json-html pulls in CIDER as a full dependency. That doesn't seem right -- CIDER should only be a dev dependency, not a packaged dependency. Here's the relevant part of pom.xml in the JAR file for 0.4.6:

  <dependencies>
    <dependency>
      <groupId>cider</groupId>
      <artifactId>cider-nrepl</artifactId>
      <version>0.21.1</version>
    </dependency>
    <dependency>
      <groupId>org.clojure</groupId>
      <artifactId>clojure</artifactId>
      <version>1.10.1</version>
      <scope>provided</scope>
    </dependency>
    <dependency>
      <groupId>org.clojure</groupId>
      <artifactId>clojurescript</artifactId>
      <version>1.10.520</version>
      <scope>provided</scope>
    </dependency>
    <dependency>
      <groupId>org.clojure</groupId>
      <artifactId>data.json</artifactId>
      <version>0.2.6</version>
    </dependency>
  </dependencies>

and here's the equivalent from 0.4.4:

  <dependencies>
    <dependency>
      <groupId>org.clojure</groupId>
      <artifactId>clojure</artifactId>
      <version>1.8.0</version>
    </dependency>
    <dependency>
      <groupId>org.clojure</groupId>
      <artifactId>clojurescript</artifactId>
      <version>1.9.521</version>
      <scope>provided</scope>
    </dependency>
    <dependency>
      <groupId>hiccup</groupId>
      <artifactId>hiccup</artifactId>
      <version>1.0.5</version>
    </dependency>
    <dependency>
      <groupId>hiccups</groupId>
      <artifactId>hiccups</artifactId>
      <version>0.3.0</version>
    </dependency>
    <dependency>
      <groupId>cheshire</groupId>
      <artifactId>cheshire</artifactId>
      <version>5.7.1</version>
    </dependency>
  </dependencies>
yogthos commented 5 years ago

That seems odd because cider isn't declared in the dependencies here.

seancorfield commented 5 years ago

It surprised me too -- and took me a while to track down why I suddenly had CIDER and nREPL in my dependencies -- but that's where clj -Stree led me and then I unpacked the pom.xml file from each of the 0.4.4 and 0.4.6 JAR files and there it was.

Is there something in your ~/.lein/profiles.clj file perhaps that's causing this, or maybe something unusual about how you built the 0.4.6 release JAR?

yogthos commented 5 years ago

I do have cider plugin in my profiles, so perhaps lein uberjar is pulling that in. Let me take a look at what's going on there.

yogthos commented 5 years ago

OK yeah so looks like having cider in :plugins of project.clj is what's causing the issue. Interestingly enough having it in ~/.lein/profiles.clj doesn't cause it to be included. I just pushed an update. Could you try it locally as a sanity check, and if everything looks good I'll push out a new version.

seancorfield commented 5 years ago

Since the problem is only going to show up in the JAR file you build (and deploy to Clojars), I'm not sure what you'd expect me to test here?

To repro the scenario where I encountered this, you'd need to build and deploy json-html 0.4.7 and then update Selmer to depend on that and deploy 1.12.14...

yogthos commented 5 years ago

Could you try building them locally using lein install to see if the right dependencies get included?

seancorfield commented 5 years ago

Looks better:

  <dependencies>
    <dependency>
      <groupId>org.clojure</groupId>
      <artifactId>clojure</artifactId>
      <version>1.10.1</version>
      <scope>provided</scope>
    </dependency>
    <dependency>
      <groupId>org.clojure</groupId>
      <artifactId>clojurescript</artifactId>
      <version>1.10.520</version>
      <scope>provided</scope>
    </dependency>
    <dependency>
      <groupId>org.clojure</groupId>
      <artifactId>data.json</artifactId>
      <version>0.2.6</version>
    </dependency>
  </dependencies>

But lein install on my machine might not do the same as lein install / lein deploy on yours :)

yogthos commented 5 years ago

Yeah, I'm seeing the same on my machine now as well. So, I'll push out new versions for both then.

yogthos commented 5 years ago

OK pushed out new versions for both Selmer and json-html, let me know if everything looks good. As a side note it looks like having the cider plugin in ~/.lein/profiles.clj doesn't cause it to be included. It's only when it's added to :plugins in projct.clj that's causing it to be pulled in.

seancorfield commented 5 years ago

Works perfectly with Selmer 1.12.14 -- Thank you!

(and this is one more reason I've switched to CLI/deps.edn for everything these days!)

yogthos commented 5 years ago

No prob, and to be fair deps has had its share of bugs as well. At least Leiningen has most of them ironed out by now through many years of use. :)

seancorfield commented 5 years ago

I like the simplicity of CLI/deps.edn. We started with Leiningen back in 2011 but switched to Boot in 2015 and then to CLI/deps.edn more recently. I've been fairly public about why we made each switch but the lack of "magic" is a contributing factor -- magic that leads to situations like this ticket :) Leiningen's plugins and profiles features are the number one thing I see beginners on Slack tripping over -- something that just doesn't happen when they learn CLI/deps.edn.

Leiningen is "easy". CLI is "simple".

Anyway, thank you for fixing this issue so quickly. I updated our primary deps.edn file to specify 1.12.14 for Selmer (we use :override-deps to manage versions) and our full test suite passes -- and our dev.repl tooling starts REBL with a Socket REPL instead of nREPL now (which is how I spotted this!). Selmer is a wonderful library that we use extremely heavily at work and I recommend it to everyone. I hadn't even seen json-html until I updated Selmer at the start of the day and tripped over the transitive CIDER dependency!

yogthos commented 5 years ago

Yeah glad we got this resolved. Amusingly enough the reason I ended up pushing new json-html was because I got rid of hiccup and hiccups dependencies to generate HTML. Turns out I traded them for cider accidentally. :)