repl: Remove magic mode

[princejwesley]

Checklist

• make -j4 test (UNIX), or vcbuild test nosign (Windows) passes
• tests and/or benchmarks are included
• documentation is changed or added
• commit message follows commit guidelines

Affected core subsystem(s)

doc, repl

Description of change

Currently magic mode retries the executed command with 'use strict'; only when the BLOCK_SCOPED_ERROR is thrown and v8 is no longer emitting BLOCK_SCOPED_ERROR.

From the v8 4.9 release note,

Classes & lexical declarations in sloppy mode
V8 has supported lexical declarations (let, const, block-local function) and classes since versions 4.1 and 4.2 respectively, but so far strict mode has been required in order to use them. As of V8 release 4.9, all of these features are now enabled outside of strict mode as well, per the ES2015 spec. This makes prototyping in the DevTools Console much easier, although we encourage developers in general to upgrade to strict mode for new code.

Also IMHO, magic mode is tightly coupled with JS engine's(v8) implementation.

[JungMinu]

IMHO, a message that indicatesmagic mode is deprecated or warning is required for compatibility

[JungMinu]

/cc @Fishrock123 PTAL

[bnoordhuis]

For completeness' sake:

Currently magic mode retries the executed command with 'use strict'; only when the BLOCK_SCOPED_ERROR is thrown and v8 is no longer emitting BLOCK_SCOPED_ERROR.

...unless you start node with --noharmony_sloppy_let.

[Fishrock123]

Not so sure about this, there could be things that we are missing, or more things like this in the future. Does it really cost us anything to keep it?

[princejwesley]

IMHO, Implementation based on v8's error message is not a good solution. We can eliminate all logic that are based on v8 error messages if we turn on multiline mode only when the user presses ctrl + enter.

Here is my concerns that I shared to @Trott

[jasnell]

While I'm OK with this in general, it's likely quite premature and could have unintended side effects down the road. Also, a deprecation cycle would be necessary before removing outright. I'd say we hold off on this for now and revisit after v7.

[ChALkeR]

Not sure why do this change.

[ChALkeR]

@princejwesley Note that v8 still throws those errors with --no-harmony-sloppy-let --no-harmony-sloppy-function parameters (as of V8 5.1 which we have in the current master).

@jasnell This introduces a (hard) deprecation cycle — it warns when the user passes magic mode and switches them to sloppy, which is equivalent now. This also intends to remove only unused code, as the idea behind this is that v8 doesn't throw errors that we are catching in our magic mode anymore. I am pretty sure this is safe to land once --no-harmony-sloppy-let and --no-harmony-sloppy-function go away.

[ChALkeR]

Hm. The deprecation cycle here is a hard deprecation. Perhaps this needs soft (i.e. documentation-only) deprecation first.

That said, probably only if V8 version in the next major won't have --no-harmony-sloppy-let and --no-harmony-sloppy-function flags. Update: it won't.

[ChALkeR]

Note: this was in fact removed even in V8 5.1 branch (in V8 5.1.297).

See v8/v8@a0a8ecd, note how it removes the error message.

That error message which we are catching in the repl magic mode is not even there anymore in V8 5.1.297+ and V8 5.2.1+.

[jasnell]

Thank you for the clarification! I'd missed that

On Sunday, July 24, 2016, Сковорода Никита Андреевич <
[email protected]> wrote:

@princejwesley https://github.com/princejwesley Note that v8 still
throws those errors with --no-harmony-sloppy-let
--no-harmony-sloppy-function parameters (as of V8 5.1 which we have in
the current master).

@jasnell https://github.com/jasnell This introduces a deprecation
cycle. This also intends to remove only unused code, as the idea behind
this is that v8 doesn't throw errors that we are catching in our magic
mode anymore. I am pretty sure this is safe to land once
--no-harmony-sloppy-let and --no-harmony-sloppy-function go away.

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#7850 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/AAa2ecn71nQktrq6dxbppk_Gb8EYLEFSks5qZCCwgaJpZM4JTTjP
.

[jasnell]

Closing in favor of #11599