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

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

Closed
zladuric opened this issue Sep 8, 2016 · 15 comments
Closed

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

zladuric opened this issue Sep 8, 2016 · 15 comments
Labels
repl Issues and PRs related to the REPL subsystem. v8 engine Issues and PRs related to the V8 dependency. wontfix Issues that will not be fixed.

Comments

@zladuric
Copy link

zladuric commented Sep 8, 2016

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 addaleax added the repl Issues and PRs related to the REPL subsystem. label Sep 8, 2016
@addaleax
Copy link
Member

addaleax commented Sep 8, 2016

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

@Fishrock123 Fishrock123 added v8 engine Issues and PRs related to the V8 dependency. wontfix Issues that will not be fixed. labels Sep 8, 2016
@Fishrock123 Fishrock123 reopened this Sep 8, 2016
@Fishrock123
Copy link
Member

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

@lance
Copy link
Member

lance commented Sep 13, 2016

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

fhinkel commented Sep 22, 2016

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

@bnoordhuis
Copy link
Member

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

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

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

pplam commented Oct 30, 2016

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

@pplam
Copy link

pplam commented Oct 30, 2016

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

@CherryDT
Copy link

CherryDT commented Jan 7, 2017

@pplam No, it's spec-compliant, see also #6118, but is problematic for a REPL environment of course...

@atul9911
Copy link

This is very weird behaviour as i cannot use that variable and can lead to memory leak as memory is already given to this obj but the compiler can not clean it.

@DinmaOtutu
Copy link

class Drone {
constructor(id, name) {
this.id = id;
this.name = name;
}
}

let drone = new Drone(A1223, flyer);
console.log('drone :' + drone.id +' ' + drone.name);
I keep getting this error "VM55:1 Uncaught SyntaxError: Identifier 'Drone' has already been declared
at :1:1" please can someone tell me why?

@CherryDT
Copy link

CherryDT commented Aug 26, 2018

@DinmaOtutu How is this related to node REPL? REPL doesn't produce errors with "VMxx" in the message. Looks like an error in a browser, not in node...

Also, I guess you already declared it further above or in a previous statement. That's what the error literally tells you.

@brentknudsenstud
Copy link

class Drone {
constructor(id, name) {
this.id = id;
this.name = name;
}
}

let drone = new Drone(A1223, flyer);
console.log('drone :' + drone.id +' ' + drone.name);
I keep getting this error "VM55:1 Uncaught SyntaxError: Identifier 'Drone' has already been declared
at :1:1" please can someone tell me why?

I'm running into the same problem in some of my code. I don't understand why it has said that Drone has already been declared when it wasn't declared as a variable. It was set as a class. Are setting a class and setting a variable both declarations?

@CherryDT
Copy link

Yes, class X {} is almost the same as const X = class {}. But this has nothing to do with this REPL-specific issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
repl Issues and PRs related to the REPL subsystem. v8 engine Issues and PRs related to the V8 dependency. wontfix Issues that will not be fixed.
Projects
None yet
Development

No branches or pull requests