Error with let: "Identifier '*' has already been declared" on REPL

[zladuric]

The problem I'm seeing is that if during declaring a variable with let something throws - that identifier is "taken", but I can't use it.

Essentially like this (stripped unnecessary stack).

let broken = JSON.parse('I am broken');
> SyntaxError: Unexpected token I
let broken = 5;
> TypeError: Identifier 'broken' has already been declared
let broken = 5;
> TypeError: Identifier 'broken' has already been declared

I'm seeing this for a long time, but never took an effort to look into it more.

Now I'm curious - is this expected behaviour? Why? If not, how can one go about and fix it?

It's on Node 5 and latest 6, but I believe I've seen it on older versions as well.

[zlatko@zlatko-desktop ~/tmp]$ node -v
v5.12.0
[zlatko@zlatko-desktop ~/tmp]$ uname -a
Linux zlatko-desktop 4.4.0-36-generic #55-Ubuntu SMP Thu Aug 11 18:01:55 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux

[addaleax]

See e.g. #8376, #8309 and probably more… unfortunately, that appears to be spec-compliant behaviour.

[Fishrock123]

I guess we may be able to provide a better error message? Maybe?

[lance]

@Fishrock123 how would you improve the error message? To me it appears pretty clear. The only thing that is really confusing is that you get two different errors depending on how you try and redefine the variable. E.g.

> let broken = foo
ReferenceError: foo is not defined
> let broken = 'foo'
TypeError: Identifier 'broken' has already been declared
> broken = 'foo';
ReferenceError: broken is not defined

Providing better error messages would mean catching all Reference errors, and comparing the error messages to these known values, then changing the message. This seems like a lot of work for something that is spec compliant.

[fhinkel]

Any objections to closing this issue since it's a wontfix?

[bnoordhuis]

Let's close, I don't think we'll fix this anytime soon.

#8309 (comment) makes an interesting suggestion of turning uninitialized bindings into undefined values but that probably needs support in V8.

[princejwesley]

@bnoordhuis We can preprocess it before passing to vm. I'll give PR if it seems good to you.

    function preprocess(code) {
       ...
        const letBinding = /^\s*let\s+([^\s=]+)\s*(?==)/;
        const letMatch = cmd.match(letBinding);
        if (letMatch && letMatch[0] !== letMatch.input) {
          const binding = letMatch[1];
          cmd = cmd.replace(letMatch[0], `let ${binding};\n${binding}`);
        }
      ...
    }

node 🙈 ₹ git:(upstream ⚡ repl.let) ./node
> let x = foo
ReferenceError: foo is not defined
    at repl:2:4
    at sigintHandlersWrap (vm.js:22:35)
    at sigintHandlersWrap (vm.js:96:12)
    at ContextifyScript.Script.runInThisContext (vm.js:21:12)
    at REPLServer.defaultEval (repl.js:313:29)
    at bound (domain.js:280:14)
    at REPLServer.runBound [as eval] (domain.js:293:12)
    at REPLServer.<anonymous> (repl.js:513:10)
    at emitOne (events.js:101:20)
    at REPLServer.emit (events.js:188:7)
> x
undefined
> x = 3
3
>

[bnoordhuis]

A bit too much of a hack, IMO. I know perfect is the enemy of good but it would fail on simple inputs like let x = 1, y = boom, let /* comment */ y = boom, f(); let y = boom, etc.

[pplam]

@lance I just caught the same issue as you in node@6.9.1 !

[pplam]

I used let, but the variable can't be reassigned. It's maybe a bug of node@6.9.1 REPL