Cannot import testdouble: rendering invalid hoisted variable

[charlag]

repo to reproduce:

https://github.com/tutao/nollup-bug/commit/9ce90c69d65835e06f65cafb1c05a08839cddcdf

(run node make)

file:///home/ivk/dev/repositories/nollup-bug/build/index.js:27235
var ;
    ^

SyntaxError: Unexpected token ';'
    at Loader.moduleStrategy (node:internal/modules/esm/translators:147:18)
    at async link (node:internal/modules/esm/module_job:64:21)

I debugged a little and it seems like it's happening around node.type === 'VariableDeclaration' because node.declarations.id doesn't have a name, it's an object pattern of some kind.

[charlag]

> JSON.stringify(node.declarations)
[
  {
    "type": "VariableDeclarator",
    "start": 2180,
    "end": 2200,
    "id": {
      "type": "ObjectPattern",
      "start": 2180,
      "end": 2187,
      "properties": [
        {
          "type": "Property",
          "start": 2182,
          "end": 2185,
          "method": false,
          "shorthand": true,
          "computed": false,
          "key": {
            "type": "Identifier",
            "start": 2182,
            "end": 2185,
            "name": "URL"
          },
          "kind": "init",
          "value": {
            "type": "Identifier",
            "start": 2182,
            "end": 2185,
            "name": "URL"
          }
        }
      ]
    },
    "init": {
      "type": "Identifier",
      "start": 2190,
      "end": 2200,
      "name": "require$$2"
    }
  }
]

[charlag]

Basically it fails on the code like var { URL } = require('url')

[PepsRyuu]

Interesting. I will have a look into it later this evening! :)

[PepsRyuu]

Was able to reproduce and created a sample test case with a few other variations. I have an idea on how to fix it, so should hopefully have a fix incoming very soon!

[charlag]

@PepsRyuu thanks for looking at it. Other than transforming it into a normal assignment without destructuring I had an idea to wrap any assignment in parens like

var blah;
// ...
({ blah } = require("..."))

this would be valid syntax but there are semicolon problems

[PepsRyuu]

I was just looking into that when you wrote that! 😁 I have the test case passing at the moment using that (with auto-insertion of the semi-colon if missing). Will need to thoroughly test it though to make sure there won't be any other issues as a result of the insertion of the new characters.

If you're curious, this is what I have so far: https://github.com/PepsRyuu/nollup/pull/227

[charlag]

@PepsRyuu yup, that's what I tried as well except I don't know magic-string well enough. I can test this tomorrow on our use case

[PepsRyuu]

Apologies for the delay, busy week. :) I've pushed an extra test case, seems to work this fix. Have you had a chance to test it with your use case? If it works for you I'll go ahead and release.

[charlag]

@PepsRyuu cheers! No worries. Didn't have a chance yet unfortunately, will try to do it on Monday

[charlag]

I'm sorry that it takes so long, I little bit busy, I will definitely do it by the end of the week