Skip to content

Improve shadow-cljs and npm interop

Isaac Johnston requested to merge superstructor/lein-shadow:master into master

lein-shadow is used by at least:

Between them re-frame and luminus have at least some thousands of users.

The lein-shadow interop story for both shadow-cljs and npm has caused issues that are at odds with upstream shadow-cljs documentation and support channels.

Ultimately, the goal of this change is to drastically reduce support requests on shadow-cljs support channels.

  1. No longer write to package.json.

Instead we call npm install --save --save-exact package@version for :npm-deps and npm-install --save-dev --save-exact package@version for :npm-dev-deps.

This is the same behavior as shadow-cljs does for transitive npm dependencies for deps.cljs files it finds on the classpath.

This change is based on a comment by Thomas Heller that by running NPM install on a freshly overwritten package.json we are undoing any work shadow-cljs has done installing transitive dependencies on every run, lowering performance.

He has also made the case that package.json should be left alone and managed by npm or yarn (not us, not shadow-cljs); e.g. npm scripts, npm outdated etc.

My testing has shown we lose nothing by interop with npm rather than package.json. Its more reliable behavior. Users get a better experience with more flexibility and access to more features.

  1. Make the comment not to edit shadow-cljs.edn really really super obvious.

Users ignore the existing comment. That is an empirical fact based on many support queries.

These have wasted Thomas Heller's time who is very active on the Clojurians #shadow-cljs support channel.

We don't want to waste Thomas Heller's time because he does an awesome job on pumping out shadow-cljs releases with shiny new features.

So lets fix this and let him focus on more important things.

  1. lein shadow help

I improved the docstring to include co-ordinates of where to go for help. This includes the shadow-cljs user guide URL and how to run the shadow-cljs cli --help via lein as it is not obvious esp to new users.

  1. README.md changes re deps.cljs

I clarified the README.md documentation on :npm-deps, deps.cljs etc. It was a feature I introduced previously, but it lacked documentation so I fixed that.

  1. jsonista is not longer a dependency

We don't need it because of 1.

  1. Run windows commands via cmd /C npm

See 7.

  1. Make NPM failures more obvious

Both 6. and 7. are based on a support request on re-frame-template https://github.com/day8/re-frame-template/issues/127

Long story short is that it was difficult for both the user, and the support (me), to determine the problem with the existing logging.

This has been fixed and tested with more robust logging.

  1. Logging of reading deps.cljs

When a deps.cljs file is found it will now log the path of the file being used.

For more background you can also see the discussion at https://github.com/day8/re-frame-template/issues/128

We have not made every single change Thomas Heller wants (notably shadow-cljs.edn is intentionally still a generated file) but we believe this will address the vast majority of support requests which is the pain point.

We also think that there are different goals here. Vanilla shadow-cljs caters more to people coming from Node.js, and that's fine. But users of lein-shadow are likely more from the Clojure ecosystem direction. Possibly with more complex needs than shadow-cljs.edn can provide, such as computation in build configuration, middleware, plugins and profiles. That is also OK. We've found a middle ground here that is sensible from both a technical design perspective and reducing upstream support requests.

Merge request reports