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

REPL emits surprising output on > {a: 1} #1421

Closed
kevinkassimo opened this issue Dec 26, 2018 · 9 comments · Fixed by #7591
Closed

REPL emits surprising output on > {a: 1} #1421

kevinkassimo opened this issue Dec 26, 2018 · 9 comments · Fixed by #7591
Labels
repl related to the Read-Eval-Print-Loop functionality of Deno

Comments

@kevinkassimo
Copy link
Contributor

kevinkassimo commented Dec 26, 2018

Currently,

> {a:1}
1
>

Expected:

> {a:1}
{ a: 1 }
>

This is due to v8 considering {} as a scope instead of object.
In Node, this is solved by secretly adding a pair of parentheses around the object, like

{a:1} => ({a:1})

The code implementing this is available here:
https://github.com/nodejs/node/blob/ae73b73eeb99101188c860cce488ccc085b2f268/lib/repl.js#L235-L243

I am expecting a similar patch for this after #1407 and #1420 are landed...

@justjavac
Copy link
Contributor

justjavac commented Dec 26, 2018

platform result
Node { a: 1 }
Chrome { a: 1 }
FF 1
Edge 1

if compatible with most platforms, it should return { a: 1 }


in Chrome repl outputs { a: 1 }. when using devtools's watch, it output 1


in Node.js

> {a:1}
{ a: 1 }

> {a:1};
1

> {a:1;}
1

> {
... a: 1
... }
{ a: 1 }

> {
... a:1;
a:1;
   ^
SyntaxError: Unexpected end of input

Chrome:

> {a:1}
{ a: 1 }

> {a:1};
1

> {a:1;}
1

> {
      a: 1
  }
{ a: 1 }

> {
      a:1;
  }
1

Should we refer to chrome? @kevinkassimo

@kevinkassimo
Copy link
Contributor Author

@justjavac Ideally we should. However, I think the behavior of Node is much easier to implement by simply introducing 2 tries, first with wrapping parentheses and second without.

On the other hand, Node internally uses acorn to do some JS parsing to determine the best handling approach. I would say we might want to keep things simpler for Deno at current stage...

@justjavac
Copy link
Contributor

keep things simpler for developers

@kevinkassimo
Copy link
Contributor Author

kevinkassimo commented Dec 26, 2018

@justjavac Eventually we should... but we would also need better infra for that... There are still a bunch of more important improvements we need to introduce to REPL... It's more about prioritization and iterations...

@Qix-
Copy link

Qix- commented Jan 22, 2020

Yeah I mean, the fact I can't even paste a Typescript function without it puking makes the REPL kind of useless for me. I find myself constantly switching back to Node and I'm only a few days into evaluating Deno.

function swap32(n: bigint): bigint {
	return ((n >> 24n) & 0xFFn) | ((n << 24n) & 0xFF000000n) | ((n & 0xFF0000n) >> 8n) | ((n & 0xFF00n) << 8n);
}


error: Uncaught SyntaxError: Unexpected token ':'
► <unknown>:1:18
    at evaluate ($deno$/repl.ts:84:34)
    at replLoop ($deno$/repl.ts:175:13)

Most of the time in the REPL I'm going to be running expressions - I want to see how the engine evaluates something.

Doing hacks is not something I'd hope Deno to do, but instead looking at the AST and figuring out whether or not the input is an expression is something I think makes a lot of sense.

@kitsonk
Copy link
Contributor

kitsonk commented Jan 22, 2020

@qix the issue you want is #1158 and it isn't so straight forward to support.

@Qix-
Copy link

Qix- commented Jan 23, 2020

I don't see how using an AST pass to determine the node type would constitute as "complex". I see it as "correct", however, and anything less correct than that feels like Deno cutting corners and going down a similar path that Node did.

Not the quality I expected from Deno. Might as well keep using Node at that point.

@kitsonk
Copy link
Contributor

kitsonk commented Jan 23, 2020

Not the quality I expected from Deno. Might as well keep using Node at that point.

Please feel free.

@kevinkassimo
Copy link
Contributor Author

kevinkassimo commented Jan 23, 2020

@Qix- Deno is not stable yet. It took Node many years to reach the quality today, and we cannot expect everything would suddenly miraculously work for Deno. (REPL is just one thing. There are many more things to worry about atm (e.g. strong Web spec compliance related work). Check PRs to see what is going on with the focus of the team)

Node is solid, and definitely continue use Node for very serious purposes for the time being.

Also I think Bartek is making some experiments right now (following the path of ts-node) #3760

@bartlomieju bartlomieju added the repl related to the Read-Eval-Print-Loop functionality of Deno label Feb 24, 2020
@ry ry closed this as completed in #7591 Sep 21, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
repl related to the Read-Eval-Print-Loop functionality of Deno
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants