Shadow CLJS does not follow standard node_module resolution

[Ionshard]

Shadow CLJS assumes that all npm dependencies are installed in ./node_modules/ but according to this guide, Node will actually climb the directory hierarchy when attempting to resolve modules.

This means Shadow CLJS can't benefit from things like hoisting.

For my case I am trying to utilize lerna now that we can use a standard package.json for our CLJS application in our monorepo.

[thheller]

It is quite intentional that it does not follow standard node resolve rules since I will never allow having multiple versions of the same dependency in one build (which happens regularly in webpack builds with nested node_modules dirs).

I'd however be open to allowing multiple search paths for packages themselves as that still wouldn't allow multiple versions of the same lib.

I however have no idea what lerna does or what hoisting means in this context. So it would help a lot if you could provide a short summary of what that is about and preferably a simple demo repo where I can see it in action.

[Ionshard]

I don't think this should cause an issue with multiple versions of the same dependency. I am not at all interested in nested node_modules folders (that is to say multiple node_modules folders below my current path) I am interested in node_modules folders above my current path.

Furthermore according to the guide linked above you only resolve a dependency from a higher path node_modules folder only if the dependency is not found from a lower path node_modules folder. This should be a deterministic algorithm that still always resolves to a single version of a dependency.

Here is a mock example, I will start building a real example shortly. Let's assume I am building a React application, I am using a monorepo so multiple packages will exist in one repo. So I have the following file structure:

lerna.config.js
package.json
packages/
  photo-component/
    package.json
    src/
  editor-component/
    package.json
    src/
  app/
    package.json
    shadow-cljs.edn
    src/

Here photo-component and editor-component are React components provided as libraries written in Javascript where our app is the final bundle written in Clojurescript using shadow-cljs.

Now using react as an example dependency (I know React is usually resolved as a peer-dependency but let's just assume it's not)

Lerna allows you to bootstrap your project, this basically just means going to each package and running npm install with some smart symlinks to your other packages.

e.g.

lerna.config.js
package.json
packages/
  photo-component/
    package.json
    node_modules/
      react/
    src/
  editor-component/
    package.json
    node_modules/
      react/
    src/
  app/
    package.json
    shadow-cljs.edn
    node_modules/
      react/
      photo-component -> ../../photo-component
      editor-component -> ../../editor-component
    src/

The symlinks are why we really want to use lerna. This works just fine with shadow-cljs currently.

However, as you can see react exists 3 times across all our packages. This is where bootstrap --hoist comes in. Using this options lerna is able to pull your common dependencies up the filesystem tree.

So instead you would have the following setup:

lerna.config.js
package.json
node_modules/
  react/
packages/
  photo-component/
    package.json
    node_modules/
    src/
  editor-component/
    package.json
    node_modules/
    src/
  app/
    package.json
    shadow-cljs.edn
    node_modules/
      photo-component -> ../../photo-component
      editor-component -> ../../editor-component
    src/

Note that the react dependency now only exists once under the root level node_modules folder.

Now when shadow-cljs goes to resolve it's react dependency you would first attempt to resolve /packages/app/node_modules/react/ but when you find that is missing then you would attempt to resolve /packages/node_modules/react/ which doesn't exists and finally you would resolve /node_modules/react which does exist and is a single version of react.

This algorithm would apply regardless of whether you were to resolve react required directly against our project, or if it was required by a dependency of our project. So if photo-component requires react you do not attempt to resolve react against /packages/photo-component/node_modules/react/ It would resolve using the exact same resolution chain as above. We always start directly at our own node_modules folder and never a nested node_modules folder, but do allow traversing up the filesystem as well.

Hopefully this clears the intended usage and benefits of hoisting a bit. I will attempt to make a simple lerna + shadow-cljs repo so we can play with a real world example of this.

[Ionshard]

Hmm, I made an example repo to show this and everything works fine. Looks like shadow-cljs is perfectly capable of resolving the dependencies using higher path node_modules folders.

However, going back to my original repository where I encountered this issue, it's actually not related to the actual module resolution. This issue actually arises when shadow-cljs is ensuring that libraries provided by :foreign-libs are resolvable. So when react isn't in our direct node_modules folder but actually higher up the filesystem, shadow-cljs is able to resolve it when I require it via ["react" :as react] just fine. But when mocking out cljsjs/react I still get the following warning.

The required JS dependency "react" is not available, it was required by "cljsjs/react.cljs".

I will try to make a demo repo that actually uses react to reproduce this.

[thheller]

Looks like shadow-cljs is perfectly capable of resolving the dependencies using higher path node_modules folders

It definitely isn't. At no point in the code does it check directories other than the configured :node-modules-dir (defaults to "node_modules").

[Ionshard]

Here is my current work on the example repo https://github.com/Kasuko/shadow-lerna-demo

Using the simple folder you can setup lerna hoisting and it seems to resolve pad-left and pad-right from simple/node_modules rather than the expected simple/packages/haunted-house/node_modules

[thheller]

You are using :node-script in that example. :node-script will just map directly to calling require which node will resolve at runtime. So there the full node resolve rules apply. No JS is touched or converted at all. It'll only attempt to look at node_modules/react to check if (:require [react]) is a npm package or not. If a string were used it would even check that step (ie. (:require ["react" :as react])).

node_modules are only processed when using :js-provider :shadow (eg. :target :browser).

[Ionshard]

Ah that makes sense, thank you. The example I am currently working on will be using :target :browser (I was just hoping to make a much simpler example) so when I am finished that I will let you know the result.

[thheller]

The example you have provided is already enough. Thank you for that. Don't need the shadow-cljs config side since I know that part. ;)

[thheller]

So far my opinion on the matter looks like this.

Looking "up" automatically is IMHO a bad idea and would probably cause issues in most projects I currently use. For example in the shadow-cljs project itself I have a few packages/thing/package.json files to separate some deps. But at the root of the project I have a dummy package.json where I install random packages for testing. I don't ever want those to "leak" into an actual build from packages/*.

I am however open to adding a new config option that would allow configuring more than one directory. Currently this is limited to one dir via :node-modules-dir. I could add something better named like :js-package-dirs ["node_modules" "../node_modules"] which should be enough to enable the lerna style setup. It would just try each entry left to right until it finds a matching package.

[Ionshard]

Ah well I already finished the browser example, so in case you're curious here is an actual react library with reagent monorepo example https://github.com/Kasuko/shadow-lerna-demo/tree/master/foreign-libs

Yes I see that lerna hoists to a single directory so the :js-package-dirs option would definitely be beneficial.

As for the example where you leak those dependencies in, it seems strange to me that shadow-cljs does want to deviate from the standard node practices. If you use that directory structure with node js it will absolutely leak those packages. Especially in the case where I am trying to combine shadow-cljs and pure JS packages, which to me was one of the big benefits of switching to shadow-cljs.

Though when it really comes down to it, I am indifferent either way. If shadow-cljs can support lerna hoisting with the :js-package-dirs I'd be a happy fellow.

[thheller]

The reasons I don't follow node resolve rules is not because of node but because of webpack. It might be ok in node to run 2 different versions of a package in a process but for the Browser this means extra bytes that need to be downloaded. Not to mention all the other possible things that can go bad.

Searching "up" might not be as bad as nested node_modules but it can still easily lead to unexpected behavior if you aren't careful, which beginners often aren't. Not supporting it to me is the safer default and has been what I wanted in 100% of my own projects (which is what I built all this for in the first place).

I can understand the use case for something like lerna so I'm willing to add :js-package-dirs.

[Ionshard]

Thank you for explaining, it definitely helps me understand how things work in Javascript, and I definitely agree with your choices. I am coming to the Javascript ecosystem from Clojure and I still barely even know what webpack actually does. So far shadow-cljs has been an amazing improvement over what we were working with before, so thank you for the work you've put into all this stuff.

[thheller]

Just released 2.8.51 which adds the mentioned :js-package-dirs.

See https://shadow-cljs.github.io/docs/UsersGuide.html#alt-node-modules

[Ionshard]

Amazing, thank you! That was so fast.

…

On Mon., Aug. 19, 2019, 3:49 p.m. Thomas Heller, ***@***.***> wrote: Just released 2.8.51 which adds the mentioned :js-package-dirs. See https://shadow-cljs.github.io/docs/UsersGuide.html#alt-node-modules — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#547?email_source=notifications&email_token=AAAVDE6HLUADHQ5XJGGCUJTQFL2LTA5CNFSM4IMMTQIKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD4UC7OQ#issuecomment-522727354>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAAVDE24IVB32OCBZOWGIRTQFL2LTANCNFSM4IMMTQIA> .