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

Lexical variable declarations (let, const) unusable in repl if error thrown during declaration/first assignment #8309

Open
polybuildr opened this issue Aug 28, 2016 · 27 comments
Labels
repl Issues and PRs related to the REPL subsystem. vm Issues and PRs related to the vm subsystem.

Comments

@polybuildr
Copy link

  • Version: v.6.3.1
  • Platform: Linux (Ubuntu 14.04)
  • Subsystem: repl (I think)
let s = Set();

gives TypeError: Constructor Set requires 'new'.

However, after this:

s

gives ReferenceError: s is not defined

and

let s = new Set();

gives TypeError: Identifier 's' has already been declared.

I'm not sure whether this is a real bug or it's expected behaviour, but it sure does seem unusual that s becomes unusable from this point in in the repl.

This isn't a problem when running node on a js file because the TypeError simply terminates execution.

@addaleax addaleax added the repl Issues and PRs related to the REPL subsystem. label Aug 28, 2016
@vsemozhetbyt
Copy link
Contributor

Different with var:

> var  s = Set();
TypeError: Constructor Set requires 'new'
> s
undefined
> var  s = new Set();
undefined
> s
Set {}

@polybuildr
Copy link
Author

Yep, works fine with var.

@vkurchatkin
Copy link
Contributor

Here is simplified test case:

var vm = require('vm');

try {
  vm.runInThisContext('let x = f');
} catch (e) {}

vm.runInThisContext('x = 1');

@addaleax addaleax added the vm Issues and PRs related to the vm subsystem. label Aug 28, 2016
@addaleax
Copy link
Member

This is very déjà-vu-ish to me, although I can’t find another issue for this bug… /cc @nodejs/v8?

@polybuildr
Copy link
Author

This error also occurs in the Chrome browser, so it's probably a v8 thing and not a Node thing. Sorry for reporting it in the wrong place.

@polybuildr
Copy link
Author

Where should I report this considering that it's not specific to node's repl?

@Fishrock123
Copy link
Contributor

I wouldn't be surprised if this was how the spec worked.

In general, you'll have a better time using var for variables in the repl.

@bnoordhuis
Copy link
Member

@polybuildr You can report it over at https://bugs.chromium.org/p/v8/issues/list. It looks spec compliant to me (the binding for s is never fully constructed and stays in the TDZ) but perhaps the error messages can be improved.

@Slayer95
Copy link

@polybuildr
Copy link
Author

@bnoordhuis, @Slayer95, it does look spec-compliant, so I'm going to close this issue. However, that esdiscuss topic pointed me to something interesting that the Firefox Devtools console does and might be worth a shot.

For any such unresolved bindings (at least in the DevTools), Firefox steps in and turns them into undefined. So a redeclaration with let doesn't work, but simply s = 5 works from that point on. If anyone would like to consider doing this for the repl, feel free to reopen the issue.

@MasterJames
Copy link

MasterJames commented Jun 9, 2018

Not working all variations

> rangeArray = nj.arange(6,12)
ReferenceError: rangeArray is not defined
> rangeArray = new nj.arange(6,12)
ReferenceError: rangeArray is not defined
> var rangeArray = new nj.arange(6,12)
SyntaxError: Identifier 'rangeArray' has already been declared
> let rangeArray = new nj.arange(6,12)
SyntaxError: Identifier 'rangeArray' has already been declared
> let rangeArray = nj.arange(6,12)
SyntaxError: Identifier 'rangeArray' has already been declared
> var rangeArray = nj.arange(6,12)
SyntaxError: Identifier 'rangeArray' has already been declared

@gireeshpunathil
Copy link
Member

re-opening, per #32288 (comment), #32288 (comment) and #32288 (comment)

@devsnek
Copy link
Member

devsnek commented May 19, 2020

This is fixable pending migration to v8 repl mode pending v8:10539

@devsnek devsnek changed the title Missing new in Set constructor makes variable defined using let unusable Lexical declarations (let, const) unusable in repl if error thrown during declaration Mar 1, 2021
@devsnek devsnek changed the title Lexical declarations (let, const) unusable in repl if error thrown during declaration Lexical variable declarations (let, const) unusable in repl if error thrown during declaration/assignment Mar 1, 2021
@devsnek devsnek changed the title Lexical variable declarations (let, const) unusable in repl if error thrown during declaration/assignment Lexical variable declarations (let, const) unusable in repl if error thrown during declaration/first assignment Mar 1, 2021
@devsnek
Copy link
Member

devsnek commented Mar 1, 2021

Just trying to make this more searchable 😇

@TheRamann

This comment was marked as off-topic.

@TiraO
Copy link

TiraO commented Dec 1, 2021

+1 this behavior is unpleasant. After making a typo, the only way to proceed is to start over if e.g. you were setting up to paste in a line of code.

@mcollina
Copy link
Member

This is fixable pending migration to v8 repl mode pending v8:10539

@devsnek is this still blocked by that issue? It was closed as wontfix.

@devsnek
Copy link
Member

devsnek commented Dec 29, 2021

@mcollina yeah we could implement repl using runtime.evaluate now, just no one has gotten around to it.

@ShogunPanda
Copy link
Contributor

@devsnek So this is a full reimplementation of the repl?

@devsnek
Copy link
Member

devsnek commented Dec 29, 2021

I don't think it requires complete reimplementation but it will definitely require some work

@ShogunPanda
Copy link
Contributor

I see.

Once I get some bandwidth I might take a look at this. This looks like and interesting challenge.

@gregfenton

This comment was marked as abuse.

@ankit142

This comment was marked as off-topic.

@gregfenton
Copy link

@ankit142 The issue as posted isn't about fixing the code in the example provided. There are many examples that could be provided.

Essentially this issue is that calling let x = <some_code_that_throws_an_error> leads to a situation where you cannot use x and you cannot redeclare let x. So for the remainder of the REPL session, the symbol x is unusable.

@ankit142

This comment was marked as off-topic.

@gregfenton
Copy link

gregfenton commented Feb 15, 2024

Again, the issue is not with "fixing the code". It is a bad behaviour of the REPL. You don't know that the code you just typed will hit an error. It does. The symbol you tried to use (x) is now unusable for the remainder of the REPL.

This isn't about "writing good code" vs. "writing bad code". This is about the user experience using the REPL when playing around with code.

  • Why would someone do this?
    • developer trying stuff -- copy/paste instructions from somewhere or whatever
  • Why not just use another symbol once x is unusable?
    • other code coming (think copy/paste instructions) leverages that same symbol; the user would be forced to either stop/restart the REPL or refactor the code they are about to paste
      • both of the workarounds above are not a good/healthy user experience

@RedYetiDev
Copy link
Member

FWIW @1Git2Clone has a issue with chromium at https://issues.chromium.org/issues/345248266.

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. vm Issues and PRs related to the vm subsystem.
Projects
None yet
Development

No branches or pull requests