Node: Global function declaration inconsistency

[ctjlewis]

• Version: v14.4.0
• Platform: Linux workspaces 5.4.0-33-generic #37-Ubuntu SMP Thu May 21 12:53:59 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux
• Subsystem:

What steps will reproduce the bug?

Global functions defined with the form f = function(){} seem to be set on the global object by default, but if the same functions are defined with the form function f(){} in the global scope, they will not be set on the global object.

Consider:

test.js

test1 = function() {}
test2 = function() {}
function test3() {}
function test4() {}

index.js

require('./test.js');
console.log(test1, test2, test3, test4);

node index.js throws an error for the function defined without variable assignment pattern:

console.log(test1, test2, test3, test4);
                          ^

ReferenceError: test3 is not defined

But test1 and test2 are fine:

console.log(test1, test2);
> [Function: test1] [Function: test2]

Expected Behavior

test3 and test4 should be available on the global object like test1 and test2.

Additional Info

Confirmed to affect the class keyword as well, i.e. TestClass = class {} will export by default but class TestClass {} will not.

[DerekNonGeneric]

Top-level functions should be stored on the globalThis object

I don't believe that is correct. Top-level functions are in the module scope, not the global scope.

[ctjlewis]

@DerekNonGeneric The issue is that output should not meaningfully differentiate between function test(){} and test = function(){}, which are syntactically equivalent. And also the fact that node -e produces different output than node file.js containing the same script to eval.

[DerekNonGeneric]

The code in file.js is a CommonJS module that has been wrapped w/ an IIFE. This makes it somewhat of module context. You can see this wrapper here.

Code passed to --eval and executed in the REPLs do not have this wrapper, and are in somewhat of a script context w/o the module scope.

[ctjlewis]

@DerekNonGeneric TY for the explanation. I'm trying to write a compatibility layer for traditional browser JS using JSDOM, which will basically generate a window and override the globalThis object when it is imported. The goal is to kind of reproduce a browser context without getting into ugly territory like reading the file contents and evaling it manually etc. - just require('enable-browser-mode') and then require('./d3.js') and so on.

Right now, by setting globalThis = window (where window is a JSDOM window), I'm able to require browser JS and execute it without errors, but there's no way to say, import a standard jQuery library and then use the $ object, because the only way a global function will be assigned to globalThis (and therefore, can be applied to window) is with the x = function(){} form or when run with node -e.

With the standard require module wrapper, it does not seem possible to get ahold of those globals without modifying the input code, which means we can no longer simply require or import a standard piece of browser script. Those global functions will just get dumped unless they're exported, and that would require changing the code and somehow manually adding every global variable to module.exports (which would be highly messy and awful).

If you have any advice on this issue, please let me know. I know I'm definitely breaking traditional module logic here, but it's actually the goal.

[ctjlewis]

@DerekNonGeneric Let me put it this way, if I have a file:

function test() {
    console.log('Test passed!');
}

As that script exists, in the browser, it could be referred to as test or window.test. When run with node test.js, without modifying the script in any way, is there some variable for which test can be accessed as a property, i.e., globalThis.test (which does not work)?

Thank you again for your help.

[DerekNonGeneric]

// test.js
global.test = function () {
  console.log('Test passed!');
};

console.log(global.test());

[DerekNonGeneric]

without modifying the script in any way

Just saw this part. It seems that you are essentially trying to create a new context, but I don't fully understand the problem. Do you want to execute the code in the file as if it were in a browser? You might need to write your own module loader w/ your own wrapper.

[ctjlewis]

Yes - effectively what I am trying to do is set global to a JSDOM window, and then require or import scripts that will set all global variables to the global variable (so test === window.test just like in the browser). It effectively means all the global variables from all imported browser scripts will exist in one large global context.

This is actually fully functional right now, if the function in test.js is defined like test = function(){}, but obviously defining functions as function test(){} is incredibly common and there is no way to "lift up" that pattern to the actual global object as far as I can tell. It seems a little strange that a syntactically insignificant change throws a wrench in the whole thing, but this is all super hacky so I am not that shocked.

Thank you again for helping me with this, any additional thoughts are welcome.

[DerekNonGeneric]

If you want to simplify your life, you could have that as test.mjs and use the various hooks that are available to a accomplish your goal.

• https://nodejs.org/api/esm.html#esm_hooks

Of particular interest may be the following.

• https://nodejs.org/api/esm.html#esm_code_getglobalpreloadcode_code_hook

Let me know if you get stuck. :)

[ctjlewis]

@DerekNonGeneric I appreciate it. I still just think it's weird that with test.js containing:

test1 = function() {}
test2 = function() {}
function test3() {}
function test4() {}

index.js containing:

require('./test.js');
console.log(test1, test2, test3, test4);

Throws an error for the function defined without variable assignment pattern:

console.log(test1, test2, test3, test4);
                          ^

ReferenceError: test3 is not defined

But test1 and test2 are fine:

console.log(test1, test2);
> [Function: test1] [Function: test2]

Should I maybe reopen this? I'm just looking for syntactic consistency here haha. If there is an intentional difference in behavior, where test = function(){} is exported by default but function test(){} is not, it will actually not be possible to build this compatibility layer [without customizing the module wrapper]. :-(

[ctjlewis]

I have decided to reopen this - I just cannot see any good reason why these two statements should produce meaningfully different output, as much as I want to believe that it's a feature and not a bug. :sweat_smile:

[DerekNonGeneric]

As I said, this is a scope issue. You never exported the functions. Try the following.

// test.js
test1 = function() {} // global.test1
test2 = function() {} // global.test2
exports.test3 = function () {} // module.exports.test3 (exported)
exports.test4 = function () {} // module.exports.test4 (exported)

// index.js
const {
  test3, // module.exports.test3 (imported from test.js)
  test4, // module.exports.test4 (imported from test.js)
} = require('./test.js');
console.log(
  test1, // global.test1
  test2, // global.test2
  test3, // module.exports.test3 (imported from test.js)
  test4, // module.exports.test4 (imported from test.js)
);

The reason for this is because function x () {} is just shorthand [exception in comment below] for var x = function () {}, which exists in the module's lexical scope, while test1 = function() {} is shorthand for global.test1 = function() {}, which attaches to the global object (global lexical scope).

[mscdex]

As was previously mentioned this is a scope issue as a declaration like function test3() {} will limit that definition to the block scope the function is in (which in this case it's just the module scope). Similarly if you did something like this:

'use strict';
{
  function test3() {}
}
test3();

or something like this:

(function() {
  function test3() {}
})();
test3();

you'd run into the same problem.

because function x () {} is just shorthand for var x = function () {}

Technically var x = function() {} will only have its variable declaration hoisted and not the assignment.

[ctjlewis]

I intentionally did not export the functions, test.js was vanillaJS and not to be changed. I understand the scope issue now: global != top-level. TY for your patience.

I don't imagine there's an object analogous to the global object which will let me access module variables? module does not seem to hold onto anything unless I set it in module.exports, and if I want to export every top-level function then I need to process the code to add every top-level function to module.exports since there's no way to actually access them all in the runtime, and if I have to do that, reformatting top-level function test(){} definitions to test = function(){} is just an efficient (and horrific) solution.

There just does not appear to be any object that stores top-level module variables the way that global variables are stored on global and globalThis. There is effectively no way to tell Node "export all the top-level variables," and there's no way to easily shoehorn this in, i.e., by appending something module.exports = module.locals to the file contents and importing that. Preprocessing of some sort would be needed to transform top-level classes and functions into global declarations or add them all to exports.

[benjamingr]

This is by-design and how the cjs module system works. Since cjs is stable I doubt this can be changed at this point and even if it could we likely wouldn't (I think?).

You can use ES modules if you want "compliant" behavior :]

[benjamingr]

I don't imagine there's an object analogous to the global object which will let me access module variables?

There isn't and this is quite intentional and by design for object-capability and sandboxing reasons reasons. As a fun fact there used to be a variable that did this (in firefox 4 I believe) called __parent__ - that has been removed for ~10 years I believe :]

There are modules on npm that let you mock out statements in the module scope by installing a require hook (like proxyquire). I would caution against them though.

[DerekNonGeneric]

it will actually not be possible to build this compatibility layer [without customizing the module wrapper]

// web-loader.mjs
/**
 * @returns {string} Code to run before application startup
 */
export function getGlobalPreloadCode() {
  return `\
const { createRequire } = getBuiltin('module');
const require = createRequire(process.cwd() + '/<preload>');

const jsdom = require('jsdom');
const { JSDOM } = jsdom;
const dom = new JSDOM('<!DOCTYPE html><p>Hello, world!</p>');
Object.defineProperties(globalThis, Object.getOwnPropertyDescriptors(dom.window));

console.log('I just set some globals!');
`;
}

This _might_ be an antipattern.

// test.mjs
console.log(document);

node --experimental-loader ./web-loader.mjs ./test.mjs

[benjamingr]

Anyway, thanks for the report. If anyone feels strongly about this feel free to reopen. I think the current behavior is by design and stable so even if we wanted to change it we can't (easily).

[ctjlewis]

Thank you for closing this, I was just about to - not a Node issue after all, and I appreciate everyone's help.

On the off-chance anyone is curious, I did find a solution: just get hands-on and use a windowRequire() function to eval the file contents, as that is effectively what is happening in the browser where there is only one context. I had never seen the (1, eval)(args) trick (calls eval in global context) until I got this function to work, so that was also pretty cool:

const fs = require('fs');
function windowRequire(file) {
    return (1,eval)(fs.readFileSync(file, 'utf-8'));
}

windowRequire('./test.js');
console.log(test1, test2, test3, test4);

> [Function: test1] [Function: test2] [Function: test3] [Function: test4]