Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Consider wrapper for setting NODE_PATH #25

Closed
borkdude opened this issue Aug 5, 2021 · 3 comments
Closed

Consider wrapper for setting NODE_PATH #25

borkdude opened this issue Aug 5, 2021 · 3 comments

Comments

@borkdude
Copy link
Collaborator

borkdude commented Aug 5, 2021

Problem

Scripts can require libraries that are local to the script, not local to the nbb npm library itself.
To accomodate this I've used createRequire with the path of the script for requiring libs, but this doesn't work for built-in libs. E.g. the built-in reagent module which is lazy loaded on demand, only loads react from the nbb install itself, not from a local library.
But node has another workaround for this, which is called NODE_PATH which allows you to set another node_modules directory to be included and this would solve the above problem.

However, I don't like writing a .js script that wraps another instance of node. This will take another 50ms or so of startup time which is not optimal.

This could be solved using a simple bash / .cmd script which wraps node but these scripts would have to do the arg parsing to decide what the node_modules relative to the script is, which is very annoying in bash.

Possible solution

So I was thinking, maybe we could write a tiny nbb wrapper in golang, Rust or Zig (even smaller) which does the arg parsing for us and then calls the nbb runner with just the args parsed as JSON + sets the correct NODE_PATH env variable.
Then the 'real' nbb doesn't have to concern itself with arg parsing and will just receive the already JSON-parsed args in an entry point function.
If we write the wrapper in GraalVM we might be able to leverage gitlibs as well and resolve deps in the CLI rather than in node (see #20) and simply pass the entire classpath to the node part, which at that point, is just a list of plain directories. This saves us a bunch of JS coding while still being able to tap into the JS ecosystem.

Alternatives

  1. Hack

Hack: https://stackoverflow.com/a/60537950/6264

This hack seems to work, e.g. when you have a /tmp/foo/node_modules with shelljs and the below script lives in /tmp/start.cljs:

module.paths.unshift('/tmp/foo/node_modules');
process.env.NODE_PATH = process.env.NODE_PATH + ':/tmp/foo/node_modules';
module.constructor._initPaths();

require("shelljs");

If we can rely on this "hack" to work in the future, then this may be preferred as it's a much simpler workaround.
The question is if loading esm modules will be affected by this hack. If not, then we still would need the wrapper.

  1. Require nbb users to use babashka and use this wrapper script:
#!/usr/bin/env bb

(require '[babashka.fs :as fs]
         '[babashka.process :as p])

(def script (first *command-line-args*))

(-> (p/process (cond-> ["nbb"]
                 script (conj script))
               (cond-> {:inherit true}
                 script
                 (assoc-in [:extra-env "NODE_PATH"]
                           ;; TODO: preserve previous NODE_PATH if populated
                           (str (fs/path (fs/parent script) "node_modules")))))
    p/check)

nil
  1. Built-in support into babashka
@borkdude
Copy link
Collaborator Author

borkdude commented Aug 5, 2021

Note that NOTE_PATH doesn't work with ESM modules. nodejs/modules#534 (comment)

So unless we migrate to`:node-library in Shadow CLJS, when it supports code splitting, this isn't really something we can do.

@borkdude borkdude closed this as completed Aug 5, 2021
@chr15m
Copy link

chr15m commented Aug 6, 2021

@borkdude an annec-data point for you. My own usage pattern with node (whether "native", shadow-cljs, or nbb) looks like this:

  • I use nvm to have a per-directory node version.
  • I only ever use libraries I have npm installed into the local node_modules folder.

@borkdude
Copy link
Collaborator Author

borkdude commented Aug 6, 2021

Thanks, so perhaps the ability of invoking scripts from other directories isn't that important then.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants