Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cannot import testdouble: rendering invalid hoisted variable #226

Open
charlag opened this issue Feb 28, 2022 · 10 comments
Open

Cannot import testdouble: rendering invalid hoisted variable #226

charlag opened this issue Feb 28, 2022 · 10 comments

Comments

@charlag
Copy link
Contributor

charlag commented Feb 28, 2022

repo to reproduce:

tutao/nollup-bug@9ce90c6

(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
Copy link
Contributor Author

charlag commented Feb 28, 2022

> 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
Copy link
Contributor Author

charlag commented Feb 28, 2022

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

@PepsRyuu
Copy link
Owner

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

@PepsRyuu
Copy link
Owner

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
Copy link
Contributor Author

charlag commented Feb 28, 2022

@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
Copy link
Owner

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: #227

@charlag
Copy link
Contributor Author

charlag commented Feb 28, 2022

@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
Copy link
Owner

PepsRyuu commented Mar 5, 2022

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
Copy link
Contributor Author

charlag commented Mar 5, 2022

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

@charlag
Copy link
Contributor Author

charlag commented Mar 14, 2022

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants