Using const or let in global scope have problem in nodejs actions

[csantanapr]

Creating an action with variables declare in the global scope using const or let
then accessing this variables inside the main produce errors.

Bug found by user @cfjedimaster:
Here is an example to reproduce:

const foo = "foo";
let bar = "bar";

function main(){

  return {
    foo: foo,
    bar: bar,
  };
}

changing to use var then it works

Running as a zip action it doesn't have this problem, but running as a simple file (i.e. eval) it has this problem.

Produce the following error

"error": "Initialization has failed due to: ReferenceError: main is not defined\n    at eval (eval at \u003canonymous\u003e (/nodejsAction/runner.js:80:57), \u003canonymous\u003e:1:1)\n    at NodeActionRunner.init (/nodejsAction/runner.js:80:45)\n    at doInit (/nodejsAction/src/service.js:134:31)\n    at initCode (/nodejsAction/src/service.js:80:24)\n    at /nodejsAction/app.js:61:13\n    at Layer.handle [as handle_request] (/nodejsAction/node_modules/express/lib/router/layer.js:95:5)\n    at next (/nodejsAction/node_modules/express/lib/router/route.js:131:13)\n    at Route.dispatch (/nodejsAction/node_modules/express/lib/router/route.js:112:3)\n    at Layer.handle [as handle_request] (/nodejsAction/node_modules/express/lib/router/layer.js:95:5)\n    at /nodejsAction/node_modules/express/lib/router/index.js:277:22"

[csantanapr]

@bjustin-ibm might be good to look into this one.

It most be something todo with on how we use eval and the scoping it creates

[psuter]

FWIW, this is "documented" here:

Note that main must be declared either as a function or as a var, due to the scoping rules for const and let which make such declarations invisible to the action runtime.

[cfjedimaster]

I'm confused - my main was a function. It is as well in the code above.

[csantanapr]

@psuter It's not very clear the statement just said that main needs to be declare as a function, which I'm already doing.

[csantanapr]

I think this is a limitation that we would end documenting, but first would like to understand the root cause on why behaves this way.

[psuter]

Sorry, I read your example too quickly. You have indeed demonstrated a more general instance of the problem than the one I wrote about in the blog post: any top-level const or let is invisible to the runtime, main is just one potentially.

[csantanapr]

I never really super fan this eval hackery it feels like magic.

I proposed we look into writing the file int tmpfs (i.e. memory) and then do a clean require("./tmpfs/action.js")

I think it's important to fix this, as I can see a lot people using let or const and maybe other context related problems.

We would need to tell folks to make a migration some how that user will need to export.main = main

OR we could try to eval, catch the exception, and then do the require procedure, for a period to allow for migration.
We could also try to append export.main = main at the end of the file

[ioana-blue]

Someone bumped into this issue and when I was asked for help, I was confused by the error as well. The example was a top-level const.

[starpit]

shouldn't we be doing the module wrappering at creation time? why wrapper it on every invocation?

we already support zip actions, and these impose an "action-as-module" requirement. so, to start with, the wsk CLI could turn any single-file javascript actions into zip actions, and upload the zip. that might solve most of the use cases, without having to mess with the invoker or any back-end bits.

(another point against eval: it obfuscates stack traces)

[csantanapr]

Good idea @starpit but I would hold on doing something in the CLI just yet.
This will leave the problem for existing actions already in the DB that are not zip.
I think in general is a good approach to eventually have all actions as zip in the DB, but would do it in a way to keep a good UX.

I think for now, we update the nodejs proxy code, to take the string, append exports.main=main; and then write to file and do a require on the file, this will work for new actions and old actions already in the DB.

Then have the cli help building the zip, and would be also good to store the zip as an attachment in the DB which @rabbah said that's the direction.
This would give the ability to do a get on the action get, and not get back a zip file.
This leaves open the ability to someone make it very fast to retrieve just the source code, but what about if I want everything from the zip except the big node_modules folder.

One crazy idea was to save the action as two attachments one for the node_modules content and a second one for anything outside, this will allow something like a Web App UI and CLI be very efficient to retrieve the source code in a browser UI and ignore the node_modules.

[tardieu]

I submitted PR #1929 to address this issue. It uses a function scope as opposed to a module to bypass the filesystem (avoid the temporary file).

[csantanapr]

@rabbah close or anything else need it? 🙏
#1929 is merged now.