Relative Javascript imports are apparently using a different CWD when run async

[aisamu]

Version

$> npm ls nbb
nbb@0.7.135

Platform

$> uname -a
Darwin aisamu 21.6.0 Darwin Kernel Version 21.6.0: Wed Aug 10 14:28:23 PDT 2022; root:xnu-8020.141.5~2/RELEASE_ARM64_T6000 arm64 arm Darwin

$> npm --version && node --version
8.19.1
v18.9.0

Problem

When invoked asynchronously, require apparently uses an incorrect cwd on relative node imports.

Reproduction

Pure Node example

// ./modules/module/helper.mjs
console.log("Loaded helper.mjs");

// ./modules/module/index.mjs
import "./helper.mjs";

console.log("Loaded index.mjs");

// ./main.mjs

// Works: Declarative sync import
// import "./modules/module/index.mjs";

// Works: Imperative async import
// import("./modules/module/index.mjs");

// Works: Imperative sync import
// await import("./modules/module/index.mjs");

// Works; Imperative delayed async import
new Promise((r) => {
  import("./modules/module/index.mjs");
});

console.log("Loaded main.mjs");

Equivalent ClojureScript Example

// ./modules/module/helper.mjs
console.log("Loaded helper.mjs");

;; ./modules/module/core.cljs
(ns modules.module.core
  (:require ["./helper.mjs" :as helper]))

(println "Loaded module" helper)

;; ./main.cljs
(ns main
  (:require [promesa.core :as p]
            ;; Works: Declarative sync import
            ;; [modules.module.core :as m]
            ))

;; Works: Imperative sync import
;; (require '[modules.module.core])

;; Fails: Imperative async import
;; It tries to load helper.mjs from '/src/helper.mjs'
;;                       instead of '/src/modules/module/helper.mjs'
(p/do (require '[modules.module.core]))

;; Fails: Imperative delayed async import
;; It tries to load helper.mjs from '/src/helper.mjs'
;;                       instead of '/src/modules/module/helper.mjs'
;; (js/Promise. #(require '[modules.module.core]))

(println "Loaded main.cljs")

Happy to provide further details/examples.

Thanks again for this amazing tool!

[borkdude]

@aisamu Thanks for the funny meme ;)

Can you make a reproduction repository or maybe a PR with a failing test?

You could also try js/import for dynamic import instead of require.

[aisamu]

Can you make a reproduction repository or maybe a PR with a failing test?

Sure!
Here it is https://github.com/aisamu/nbb-relative-async-imports

You could also try js/import for dynamic import instead of require.

Oh, can I require cljs modules directly with js/import?
I'll give that a try later this week, thanks!

[borkdude]

Oh sorry, I mixed up js/require and require, 🤦 No you can't do that.

I'll take a look later this week at your repro.

[borkdude]

@aisamu A potential solution:

(ns main
  (:require [promesa.core :as p]
            [nbb.core :refer [await]]))

;; It tries to load helper.mjs from '/src/helper.mjs'
;;                       instead of '/src/modules/module/helper.mjs'
(await (js/Promise. #(require '[modules.module.core])))

So await the promise to be resolved. I think the issue is that the require gets execute outside of the namespace bindings.
Would this be a solution?

[aisamu]

Thanks! I've updated the repo with two new entries:

• The one above, which half-works and stops executing code after the await:

  $> nbb src/main-imp-del-async-awaited.cljs
  ;; Half-works: Imperative promise-delayed async import awaited
  Loaded helper.mjs
  Loaded module
  
  ;; Where is "Loaded main.cljs?" 
  

• Awaiting the existing p/do example, which works!:

  $> nbb src/main-imp-async-awaited.cljs
  ;; Works: Imperative do-delayed async import awaited
  Loaded helper.mjs
  Loaded module
  Loaded main.cljs
  

Would this be a solution?

Apart from the above Promise vs p/do issue, perhaps it would be helpful to await on our behalf behind the scenes.
The failure mode felt a bit unorthodox to me, and tracing it back to the root cause took a little while.
It was a bit surprising for the effect of failing to await a promise at the right time to manifest as different results (vs pending results)!

[borkdude]

It’s a known problem that using non-top-level (async) requires don’t work well in nbb but this could potentially be fixed. So far I haven’t had many use cases for this myself but perhaps you could clarify yours.

On Thu, 29 Sep 2022 at 14:41, Samuel Tschiedel ***@***.***> wrote: Thanks! I've updated the repo <aisamu/nbb-relative-async-imports@8fe7ce2> with two new entries: - The one above, which half-works and stops executing code after the await: $> nbb src/main-imp-del-async-awaited.cljs ;; Half-works: Imperative promise-delayed async import awaited Loaded helper.mjs Loaded module ;; Where is "Loaded main.cljs?" - Awaiting the existing p/do example, which works!: $> nbb src/main-imp-async-awaited.cljs ;; Works: Imperative do-delayed async import awaited Loaded helper.mjs Loaded module Loaded main.cljs Would this be a solution? Apart from the above Promise vs p/do issue, perhaps it would be helpful to await on our behalf behind the scenes. The failure mode felt a bit unorthodox to me, and tracing it back to the root cause took a little while. It was a bit surprising for the effect of failing to await a promise at the right time to manifest as different results (vs pending results)! — Reply to this email directly, view it on GitHub <#258 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AACFSBUDDOBQRWSGZOTKVELWAWE7XANCNFSM6AAAAAAQVKYZMY> . You are receiving this because you commented.Message ID: ***@***.***>

-- https://www.michielborkent.nl https://www.eetvoorjeleven.nu

[aisamu]

Perhaps you could clarify yours.

Definitely!
I have a generic runner and many small different "modules".
The goal was to be able to add modules "dynamically" just by adding files on a specific folder.

The following is a stripped-down view.
I'd run the "example" module by invoking:

nbb main.cljs example

main.cljs:

(ns main 
  [...])

(defn module-namespace [name]
  (str/join "." ["modules" name "core"]))

(defn module-symbol [name symbol-name]
  (some-> (module-namespace name)
         (symbol  symbol-name)
          resolve
          deref))

(defn load-module [name]
  (require [(symbol (module-namespace name))]))

(defn run-module [module]
  (p/do
    (defp system (irrelevant-setup))
    (defp results ((module-symbol module "run") system))
    (println "Completed" results)))

(let [name (first *command-line-args*)]
  (p/do
    (load-module name)
    (run-module name)))

The symbol-resolve-deref mechanism was the only way I could find to reach into the dynamically loaded namespace.
It feels a tad clunky and I'm more than ready to be told there's simpler way if only I had RTFM!

modules/example/core.cljs

(ns modules.example.core
  (:require
   ["./setup.mjs$default" :as recorded-setup])) ;; <- The source of all my issues :)

(defn run [system]
    (recorded-setup) 
    [...])

modules/example/setup.mjs

import "irrelevant"
// multiple tedious steps generated by another tool

Thanks for your attention and patience on this!

[borkdude]

Is this repo public maybe?

[aisamu]

It isn't, but I can convert the example above into one if you prefer!

[borkdude]

I think your original repro is sufficient to exhibit the problem, so I think I'm good for now. I'll have a closer look.

[borkdude]

What I also discovered is that when you move the println in main-imp-async.cljs within the p/do which you probably should, then it also works correctly:

(p/do (require '[modules.module.core])
      (println "Loaded main.cljs"))

$ nbb --classpath /tmp/nbb-relative-async-imports/src /tmp/nbb-relative-async-imports/src/main-imp-async.cljs
Loaded helper.mjs
Loaded module
Loaded main.cljs

So instead of firing the promise and then doing something else, it's probably better to chain the actions after the previous promise has been resolved.

[borkdude]

Instead of in JS:

import("./modules/module/index.mjs");

console.log("Loaded main.mjs");

I would also write:

await import("./modules/module/index.mjs");

console.log("Loaded main.mjs");

since you can't state that the JS file was loaded before that promise has been resolved.

[aisamu]

Instead of in JS:

Those imperative import(...) calls were only added to investigate how they operate on regular node!

On the "real" project all the js imports are done by cljs, on the respective the modules.<module>.core namespace!

Either way, it may be worth noting that the non-awaited import(...) behaves "correctly" on regular node.:

# await import(...)
$> node src/main-imp-sync.mjs
Loaded helper.mjs
Loaded index.mjs
Loaded main.mjs

# import(...)
$> node src/main-imp-async.mjs
Loaded main.mjs
Loaded helper.mjs
Loaded index.mjs

# Promise(()=> import(...)
$> node src/main-imp-del-async.mjs
// Works; Imperative delayed async import
Loaded main.mjs
Loaded helper.mjs
Loaded index.mjs

The print order changes when you await the improts (understandable) but they eventually show up!

The fact that they never show up on nbb on the equivalent cljs version was surprising:

# (await (js/Promise. #(require '[modules.module.core])))
$> nbb src/main-imp-del-async-awaited.cljs
Loaded helper.mjs
Loaded module

# Where is "Loaded main.cljs?" 

[borkdude]

I'm still looking into a fix. "It's complicated".

[aisamu]

I'm still looking into a fix. "It's complicated".

😄 I truly believe you. It would be unwise not to trust a former real-rock-star now also-programming-rock-star.

When you move the println in main-imp-async.cljs within the p/do which you probably should, then it also works correctly:

Confirmed! And surprising!

So instead of firing the promise and then doing something else, it's probably better to chain the actions after the previous promise has been resolved.

I don't recall it perfectly, so please take the following with a huge grain of salt:

I think I tried awaiting(p/do) everything I could see in front of me when I first saw this issue and the problem still persisted.
What could have happened is that I forgot to await one arbitrary level and that sufficed to let the require promise "spin freely".
If that's true, I bet I'd still run into this issue even while trying my best to apply the workaround due to sheer incompetence!
</handwavy-hypothesis>

[borkdude]

@aisamu I think I have a solution in #261

Your example works locally for me now. I think I'll have to convert this into a test still.

Are you able to build and test this PR locally?

[borkdude]

How I tested this locally:

bb dev

for building nbb.

Then from the nbb dir:

$ ./cli.js --classpath /tmp/nbb-relative-async-imports/src /tmp/nbb-relative-async-imports/src/main-imp-async.cljs
Loaded helper.mjs
Loaded module
Loaded main.cljs

[aisamu]

Updated the reproduction repository with 1.0.136 and all the relative imports now work beautifully 🥳

Thanks a lot!