Fix circular hoisting for variable patterns

[charlag]

I tried it out and got this error:

nollup-int:///../libs/cborg.js:122
    ( useBuffer = globalThis.process && !globalThis.process.browser && globalThis.Buffer && typeof globalThis.Buffer.isBuffer === 'function');
    ^

TypeError: Class constructor Token cannot be invoked without 'new'
    at eval (nollup-int:///../libs/cborg.js:122:5)

I don't see where exactly it is yet

[charlag]

I think the problem is that you should put semicolon before the parens, otherwise javascript happens:

Token = class Token {

  constructor(type, value, encodedLength) {

    this.type = type;

    this.value = value;

    this.encodedLength = encodedLength;

    this.encodedBytes = undefined;

  }

  toString() {

    return `Token[${ this.type }].${ this.value }`;

  }

}

    ( useBuffer = globalThis.process && !globalThis.process.browser && globalThis.Buffer && typeof globalThis.Buffer.isBuffer === 'function');

it tries to call the previous class here

I would generate like ;(useBuffer = ... )

[PepsRyuu]

Great catch, let me try reproduce it and see what I can do!

[PepsRyuu]

Pushed a fix using the semi-colon prefix suggestion with an updated test.

[charlag]

Hey I think importing testdouble still does not work for us:

nollup-int:///../node_modules/testdouble/lib/index.js:50
    function: function_1.default,
                         ^

TypeError: Cannot read property 'default' of undefined
    at eval (nollup-int:///../node_modules/testdouble/lib/index.js:50:26)
    at file:///home/ivk/dev/repositories/tutanota-3/test/build/Suite.js:88215:21

(which is from lodash I think)

[PepsRyuu]

Do you have an exact replica of that issue? Using the original code presented, I'm getting a different error entirely (which seems to be a compatible issue with @rollup/plugin-commonjs overriding module unexpectedly or something related to importing Node's module builtin).

[PepsRyuu]

Just for reference, I think there's another issue happening here related to how defaults work with CJS.

The code here is trying to do something like Module.default.default. Seems to work with Rollup, but I think this is unrelated to live-bindings and is something to do with recognising how NodeJS handles CJS imported into ESM.

[charlag]

How I would love for people to start actually writing ESM code...

I can tell you how to reproduce it with our project but it will be hard to debug probably.

Do you have any suggestion how to make testdouble work for us?

[PepsRyuu]

I have a bit of free time to look into this, seems like it could potentially be a growing problem. Can you confirm that it is working with Rollup? There also seems to be a dist version of testdouble, have you tried using @rollup/plugin-alias and pointing it to that file directly?

[PepsRyuu]

Looks like I was mistaken thinking it works with Rollup, it doesn't as far as I can tell. Fails to find the import due to the .default.default issue. If I import testdouble natively inside Node it will work just fine because it has a different way of resolving CJS.

[charlag]

@PepsRyuu thanks, I did not try the dist version yet. Thanks for checking with Rollup. Perhaps I should report it to plugin or testdouble, not sure

[PepsRyuu]

Looks like there's a flag on the CJS for this exact problem and it works.

[charlag]

@PepsRyuu it might break some other cases but I will try that soon, thanks!

[PepsRyuu]

Pushed an update, fixing other issues that caused this to break, namely the module shadowing, and synthetic exports not checking if the property was already exported.

[charlag]

Somehow 3f1c0b1 and defaultIsModuleExports: true (w/ plugin-commonjs 18.1.0) didn't help, same error.

I will probably come back to it when I am back from holiday.