# Adding support for installing ESM module nodes

**URL:** <https://discourse.nodered.org/t/adding-support-for-installing-esm-module-nodes/81438>\
**Category:** Core Development\
**Created:** [19 September 2023 17:25 UTC](https://discourse.nodered.org/t/adding-support-for-installing-esm-module-nodes/81438 "2023-09-19T17:25:07Z")\
**Posts on this page:** 12\
**Page:** 1

<div class="post-metadata">

**Author:** ![hlovdal](https://sea2.discourse-cdn.com/flex026/user_avatar/discourse.nodered.org/hlovdal/32/83012_2.png) [@hlovdal](https://discourse.nodered.org/u/hlovdal)\
**Post date:** [19 September 2023 17:25 UTC](https://discourse.nodered.org/t/adding-support-for-installing-esm-module-nodes/81438/1 "2023-09-19T17:25:07Z")

</div>

Adding support for installing ESM module nodes

I am working on fixing node-red to not fail when attempting to install a node written as an ESM module, e.g.

```
19 Sep 18:59:53 - [info] Server now running at http://127.0.0.1:1880/
19 Sep 18:59:53 - [info] Starting flows
19 Sep 18:59:53 - [info] Started flows
19 Sep 19:01:05 - [info] Installing module: @hlovdal/node-red-lowercase-in-typescript, version: 1.0.0
19 Sep 19:01:12 - [info] Installed module: @hlovdal/node-red-lowercase-in-typescript
19 Sep 19:01:12 - [info] Added node types:
19 Sep 19:01:12 - [info] - @hlovdal/node-red-lowercase-in-typescript:lower-case : Error [ERR_REQUIRE_ESM]: require() of ES Module .../.node-red/node_modules/@hlovdal/node-red-lowercase-in-typescript/src/lower-case.js from .../node-red/packages/node_modules/@node-red/registry/lib/loader.js not supported.
Instead change the require of lower-case.js in .../node-red/packages/node_modules/@node-red/registry/lib/loader.js to a dynamic import() which is available in all CommonJS modules.

```

Tested with 3.1.0 (and master branch as of writing this (although not that many differences)).

I have a solution and I am ready to create a pull request. The contributing guidelines strongly suggest to discuss here first.

The fix itself is rather simple, just using a dynamic `import()` instead of `require`.

But one question I have is regarding the content of the pull request: while working on this I have found and fixed a few other minor, unrelated issues like removing unused variables/functions. Can I include those in the same pull request or should I split the branch and make one pull request that purely fixes ESM support and another one with minor code improvements?

---

<div class="post-metadata">

**Author:** ![knolleary](https://sea2.discourse-cdn.com/flex026/user_avatar/discourse.nodered.org/knolleary/32/3_2.png) [@knolleary](https://discourse.nodered.org/u/knolleary)\
**Post date:** [19 September 2023 20:25 UTC](https://discourse.nodered.org/t/adding-support-for-installing-esm-module-nodes/81438/2 "2023-09-19T20:25:22Z")

</div>

> [@hlovdal](#):
>
> But one question I have is regarding the content of the pull request: while working on this I have found and fixed a few other minor, unrelated issues like removing unused variables/functions.

It would certainly make it easier if you could split them out.

Please also target the `dev` branch initially - this is for new feature development which this would be.

---

<div class="post-metadata">

**Author:** ![hlovdal](https://sea2.discourse-cdn.com/flex026/user_avatar/discourse.nodered.org/hlovdal/32/83012_2.png) [@hlovdal](https://discourse.nodered.org/u/hlovdal)\
**Post date:** [19 September 2023 20:26 UTC](https://discourse.nodered.org/t/adding-support-for-installing-esm-module-nodes/81438/3 "2023-09-19T20:26:33Z")

</div>

Ok, I'll split the branch into two PRs.

---

<div class="post-metadata">

**Author:** ![TotallyInformation](https://sea2.discourse-cdn.com/flex026/user_avatar/discourse.nodered.org/totallyinformation/32/31_2.png) [@TotallyInformation](https://discourse.nodered.org/u/TotallyInformation)\
**Post date:** [20 September 2023 14:32 UTC](https://discourse.nodered.org/t/adding-support-for-installing-esm-module-nodes/81438/4 "2023-09-20T14:32:05Z")

</div>

> [@hlovdal](#):
>
> The fix itself is rather simple, just using a dynamic `import()` instead of `require`.

Though dynamic imports are async, so how is that being handled?

Be fantastic if this could be solved in a robust way.

---

<div class="post-metadata">

**Author:** ![knolleary](https://sea2.discourse-cdn.com/flex026/user_avatar/discourse.nodered.org/knolleary/32/3_2.png) [@knolleary](https://discourse.nodered.org/u/knolleary)\
**Post date:** [20 September 2023 16:03 UTC](https://discourse.nodered.org/t/adding-support-for-installing-esm-module-nodes/81438/5 "2023-09-20T16:03:34Z")

</div>

> [@TotallyInformation](#):
>
> Though dynamic imports are async, so how is that being handled?

The node loading code inside the runtime is already async, even if the current call to `require` is sync. So this switch is actually pretty transparent to everything else.

---

<div class="post-metadata">

**Author:** ![TotallyInformation](https://sea2.discourse-cdn.com/flex026/user_avatar/discourse.nodered.org/totallyinformation/32/31_2.png) [@TotallyInformation](https://discourse.nodered.org/u/TotallyInformation)\
**Post date:** [20 September 2023 16:41 UTC](https://discourse.nodered.org/t/adding-support-for-installing-esm-module-nodes/81438/6 "2023-09-20T16:41:57Z")

</div>

> [@knolleary](#):
>
> The node loading code inside the runtime is already async, even if the current call to `require` is sync. So this switch is actually pretty transparent to everything else.

Nice, good to hear. Afraid I've not had a chance to look at that part of the code. I'll watch out for the PR. This is something I've been hoping someone could solve a while.

I need to do some more refactoring of some of my code in readiness of future moves in this direction.

Already starting to work on a new module so as to move all the file system handling to a single promises-only based class and to remove the dependency on the `fs-extra` library. But it is quite a bit of work and probably wont be complete for quite a while.

---

<div class="post-metadata">

**Author:** ![hlovdal](https://sea2.discourse-cdn.com/flex026/user_avatar/discourse.nodered.org/hlovdal/32/83012_2.png) [@hlovdal](https://discourse.nodered.org/u/hlovdal)\
**Post date:** [20 September 2023 21:23 UTC](https://discourse.nodered.org/t/adding-support-for-installing-esm-module-nodes/81438/7 "2023-09-20T21:23:45Z")

</div>

I noticed that one of the CI builds [failed](https://github.com/node-red/node-red/actions/runs/6241210307/job/16954973217?pr=4355):

```plaintext
> Use Node.js 16

Run actions/setup-node@v3
Found in cache @ /opt/hostedtoolcache/node/16.20.2/x64
Environment details

...
> Run tests
...
  2450 passing (2m)
  59 pending
  1 failing

  1) function node
       init function
         should delay handling messages until init completes:
     Error: Message received before init completed - was 191 expected >300
      at Object._inputCallback (test/nodes/core/function/10-function_spec.js:1738:34)
      at /home/runner/work/node-red/node-red/packages/node_modules/@node-red/runtime/lib/nodes/Node.js:76:33
      at Object.trigger (packages/node_modules/@node-red/util/lib/hooks.js:44:1839)
      at Object.Node._emitInput (packages/node_modules/@node-red/runtime/lib/nodes/Node.js:75:30)
      at Object.Node.emit (packages/node_modules/@node-red/runtime/lib/nodes/Node.js:70:240)
      at Object.Node.receive (packages/node_modules/@node-red/runtime/lib/nodes/Node.js:122:395)
      at deliverMessageToDestination (packages/node_modules/@node-red/runtime/lib/flows/Flow.js:164:1119)
      at Immediate._onImmediate (packages/node_modules/@node-red/runtime/lib/flows/Flow.js:165:466)
      at processImmediate (node:internal/timers:466:21)

Warning: Task "simplemocha:all" failed.a Use --force to continue.

Aborted due to warnings.

```

I am not able to reproduce this locally, installing nodejs 16.20.2 with nvm.

But looking into the stacktrace there seems to be something fishy going on because line 76 of Node.js is in the middle of a comment on my branch (with no changes of this file compared to `dev`):

```bash
$ cat -n packages/node_modules/@node-red/runtime/lib/nodes/Node.js | sed -n 70,80p 
    70
    71 util.inherits(Node, EventEmitter);
    72
    73 /**
    74 * Update the wiring configuration for this node.
    75 *
    76 * We try to optimise the message handling path. To do this there are three
    77 * cases to consider:
    78 * 1. this node is wired to nothing. In this case we replace node.send with a
    79 * NO-OP function.
    80 * 2. this node is wired to one other node. In this case we set `this._wire`
$

```

Line 76 has an offset of one line compared to the `master` branch, so any stale checkout from there is not the cause either...

Any ideas?

---

<div class="post-metadata">

**Author:** ![Steve-Mcl](https://sea2.discourse-cdn.com/flex026/user_avatar/discourse.nodered.org/steve-mcl/32/4826_2.png) [@Steve-Mcl](https://discourse.nodered.org/u/Steve-Mcl)\
**Post date:** [20 September 2023 21:32 UTC](https://discourse.nodered.org/t/adding-support-for-installing-esm-module-nodes/81438/8 "2023-09-20T21:32:07Z")

</div>

> [@hlovdal](#):
>
> `(test/nodes/core/function/10-function_spec.js:1738:34)`

The error is occurring here ☝

What is on that line in the test?

It certainly looks like a timing error

> should delay handling messages until init completes

---

<div class="post-metadata">

**Author:** ![hlovdal](https://sea2.discourse-cdn.com/flex026/user_avatar/discourse.nodered.org/hlovdal/32/83012_2.png) [@hlovdal](https://discourse.nodered.org/u/hlovdal)\
**Post date:** [20 September 2023 21:40 UTC](https://discourse.nodered.org/t/adding-support-for-installing-esm-module-nodes/81438/9 "2023-09-20T21:40:48Z")

</div>

```bash
$ cat -n test/nodes/core/function/10-function_spec.js | sed -n 1738p
  1738 done(new Error(`Message received before init completed - was ${msg.delta} expected >300`))
$

```

So at least that makes sense with regards to line numbers.

---

<div class="post-metadata">

**Author:** ![hlovdal](https://sea2.discourse-cdn.com/flex026/user_avatar/discourse.nodered.org/hlovdal/32/83012_2.png) [@hlovdal](https://discourse.nodered.org/u/hlovdal)\
**Post date:** [20 September 2023 21:54 UTC](https://discourse.nodered.org/t/adding-support-for-installing-esm-module-nodes/81438/10 "2023-09-20T21:54:34Z")

</div>

The test is the following.

```javascript
var functionNode = require("nr-test-utils").require("@node-red/nodes/core/function/10-function.js");
...
var helper = require("node-red-node-test-helper");
...
        it('should delay handling messages until init completes', function(done) {
            var flow = [{id:"n1",type:"function",wires:[["n2"]],initialize: `
                return new Promise((resolve,reject) => {
                    setTimeout(resolve,200)
                })`,
                func:"return msg;"
            },
            {id:"n2", type:"helper"}];
            helper.load(functionNode, flow, function() {
                var n1 = helper.getNode("n1");
                var n2 = helper.getNode("n2");
                var receivedMsgs = [];
                n2.on("input", function(msg) {
                    msg.delta = Date.now() - msg.payload;
                    receivedMsgs.push(msg)
                    if (receivedMsgs.length === 5) {
                        var errors = receivedMsgs.filter(msg => msg.delta < 200)
                        if (errors.length > 0) {
                            done(new Error(`Message received before init completed - was ${msg.delta} expected >300`))
                        } else {
                            done();
                        }
                    }
                });
                for (var i=0;i<5;i++) {
                    n1.receive({payload: Date.now(),topic: "msg"+i});
                }
            });
        });

```

So the threshold is 200ms and not 300 as the error indicates (and 191 is not that far off from that).

I pushed a new commit to trigger a re-rest.

* * *

By editing `Gruntfile.js` to only test this file

```javascript
         simplemocha: {
            ...
            all: { src: ["test/nodes/core/function/10-function_spec.js"]},
            core: { src: ["test/nodes/core/function/10-function_spec.js"]},
            nodes: { src: ["test/nodes/core/function/10-function_spec.js"]},
        },

```

and then running several times I was able to reproduce once:

```plaintext
  1) function node
       init function
         should delay handling messages until init completes:
     Error: Message received before init completed - was 199 expected >300
      at Object._inputCallback (test/nodes/core/function/10-function_spec.js:1738:34)

```

---

<div class="post-metadata">

**Author:** ![hlovdal](https://sea2.discourse-cdn.com/flex026/user_avatar/discourse.nodered.org/hlovdal/32/83012_2.png) [@hlovdal](https://discourse.nodered.org/u/hlovdal)\
**Post date:** [21 September 2023 18:47 UTC](https://discourse.nodered.org/t/adding-support-for-installing-esm-module-nodes/81438/11 "2023-09-21T18:47:45Z")

</div>

I did some overnight testing and the test failure is an existing problem that applies to both `dev` and `master`. The test fails around 7% of the time. Details in [github comment](https://github.com/node-red/node-red/pull/4355#issuecomment-1730114479).

---

<div class="post-metadata">

**Author:** ![system](https://us1.discourse-cdn.com/flex026/uploads/nodered/original/1X/d073cd938eafa2e558d7c2cd59003b3ef4963033.png) [@system](https://discourse.nodered.org/u/system)\
**Post date:** [20 November 2023 18:48 UTC](https://discourse.nodered.org/t/adding-support-for-installing-esm-module-nodes/81438/12 "2023-11-20T18:48:45Z")

</div>

This topic was automatically closed 60 days after the last reply. New replies are no longer allowed.
