Njs: Segmentation violation while importing

[johnpantini]

The following code causes njs to crash with a segfault:

handler.js

import Blob from 'blob.js';

export default function () {
  new Blob();
}

blob.js

var blobSupported = false;

try {
  blobSupported = new Blob(['ä']).size === 2;
} catch (e) {
}

export default {};

[xeioex]

@vryabinin

Take a look at compatibility page.

Currently only "default export and default import statements" are supported.

export default function ... expression is not supported yet. See https://github.com/nginx/njs/issues/121

[johnpantini]

@xeioex

It shouldn't cause a segv anyway. It compiles OK and doesn't produce a warning ("Illegal export statement") since handler.js is being imported itself. The segv disappears when I remove these lines:

var blobSupported = false;

try {
  blobSupported = new Blob(['ä']).size === 2;
} catch (e) {
}

[xeioex]

@vryabinin

It shouldn't cause a segv anyway

agree. Will be fixed.

[hongzhidao]

@xeioex

I simplified it again.

# blob.js
new Blob();
export default 1;

# handler.js
import Blob from 'blob.js';
export default 1;

#segv.js
import handler from 'handler.js';

The question is the imported file referenced outside variables. Here is Blob.
As expected, they are not able to access these outside variables because of hoist.
But, the name of import is processed as a variable. It has been added to the current scope.
So blob.js can access the variable Blob. But I think it's not allowed.

1. Help confirm that only global (I mean those are predefined in language, such as Object, Array, etc) and local(declared in modules) variables are allowed to access in modules. @drsm
   https://exploringjs.com/es6/ch_modules.html#_benefit-variable-checking
2. I suggest fixing it after parser refactoring.

[drsm]

@hongzhidao

Help confirm that only global (I mean those are predefined in language, such as Object, Array, etc) and local(declared in modules) variables are allowed to access in modules. @drsm

Yes. Module scopes are independent.

In our case let will help to detect collisions:

/*
$ cat a.js
console.log(name_from_b);
export default 1;
$ cat b.js
import name_from_b from 'a.js';
export default 1;
$ cat c.js
import name from 'b.js'
*/

var name = (function b() {
    var name_from_b = (function a() {
        console.log(name_from_b);
        return 1;
    })();
    return 1;
})();

// vs.

let x_name = (function b() {
    let x_name_from_b = (function a() {
        console.log(x_name_from_b);
        return 1;
    })();
    return 1;
})();

[xeioex]

@hongzhidao

I suggest fixing it after parser refactoring.

Agree.

[hongzhidao]

@xeioex

In our case let will help to detect collisions

BTW, this seems a good idea though let is hard to implement.
If we fixed this issue based on the current parser, it may be not a good idea.

[hongzhidao]

@drsm
Help to take a look at the example.

import * as os from 'os';
var a = 1;
console.log(globalThis.a);

In babel, it shows 1.
In QuickJS, it shows undefined.

Which is correct?

[drsm]

@hongzhidao

The latter is correct.
Related to #115

[hongzhidao]

@drsm
I get it, now. Thanks.
But what's the way of getting global variables?

[drsm]

@hongzhidao

global variables work same way in both scripts or modules:

$ cat x.mjs 
import * as y from './y.mjs';

console.log(test, y.test, globalThis.test);
$ cat y.mjs 
var test; // module scope

globalThis.test = 'xxx';

export { test };
$ node --experimental-modules x.mjs 
(node:8744) ExperimentalWarning: The ESM module loader is experimental.
xxx undefined xxx

So, globalThis is a hacky thing.

[xeioex]

A related issue: https://github.com/nginx/njs/issues/371