repl: unflag --experimental-repl-await

[guybedford]

This enables the --experimental-repl-await flag automatically.

Personally I find the await import('module') pattern indispensible here and keep forgetting that the --experimental-repl-await flag exists. This really makes modules experimentation much much easier, amongst many other things.

I would propose we keep the feature itself classed as unstable, but the REPL should in theory be able to accommodate more churn than most outward Node.js APIs anyway.

//cc @nodejs/modules @nodejs/repl @devsnek

[devsnek]

I think we should use the debugger repl api instead of unflagging this.

[guybedford]

@devsnek what is that?

[devsnek]

@guybedford it's a mode in v8 that allows redeclaration of let and top level await. I wrote a repl using it here: https://github.com/nodejs/repl

[guybedford]

@devsnek have you considered upstreaming this?

[devsnek]

yeah, I just never find the time to write tests and such.

[aduh95]

Duplicate of #34733?

[guybedford]

@devsnek I definitely agree it would be ideal to land the new repl project, but given we don't know how much longer this will take, it may still be worth landing this to benefit current users.

Would you still object to this as a temporary measure? What can be done to improve progress on the new repl work?

[devsnek]

I think a better short term project would be using the repl inspector api in our current repl code, not unflagging this.

[guybedford]

@devsnek I agree it would be better to land the repl inspector api but we must be weary of hampering user workflows today as well unnecessarily.

Specifically are there any reasons not to unflag this for users? If so, could you clarify what you see those as being?

Then in terms of moving the repl inspector api forward, what can be done to help with this process? Are there specific tracking issues remaining here?

[devsnek]

Specifically are there any reasons not to unflag this for users? If so, could you clarify what you see those as being?

it just randomly breaks stuff:

For the specific use case of a repl, breaking on odd inputs seems especially bad.

Then in terms of moving the repl inspector api forward, what can be done to help with this process? Are there specific tracking issues remaining here?

If you mean integrating into the existing code, @BridgeAR probably knows the most about that.

[guybedford]

try { await x() } catch (e) { console.log(e.stack) } still works fine - this case does not seem like a dealbreaker to me.

The await Promise..resolve() case doesn't seem ideal yes. Perhaps we can look into fixing this?

Do you have that shortlist of issues for upstreaming the new repl project? I think clarifying the roadmap would help get others on board with this process and potentially get you some help on it too.

[devsnek]

@guybedford maybe just rolling it up and adding it as a dependency? someone mentioned the lack of tests made them uneasy. i'm not sure what other requirements collabs might have.

this case does not seem like a dealbreaker to me.

In the context of a repl i think it is. It isn't even immediately clear if this is due to the completion value getting lost or because it somehow didn't throw an exception. A repl you can't trust the output of is kind of useless imo.

[guybedford]

Do you have a link to the previous tests comment?

IMO it is far far more important to support the native module system in the REPL than it is to have perfect repl completions. Most JS developers aren't even aware of these completion value details anyway.

I'm going to see if I can PR a fix to output the acorn parse error when there are await parsing errors. Let me know if that might work for you here.

[guybedford]

@devsnek I've posted a fix to your first issue in #39154. Assuming that lands, is there anything else specific you see as a concern for landing this PR?

[devsnek]

i dunno, i'm sure if i used it more i could find more broken stuff. i'm not sure it is a helpful exercise either way.

[guybedford]

@devsnek it's certainly not an ideal approach, but this is about getting something out. Perfect and good and all. Can you try and let me know if there is anything else you'd specifically want to block this PR on?

[devsnek]

Could we use regenerator transpiler logic or something to handle statements better? Losing the return value kinda sucks.

[guybedford]

Could we use regenerator transpiler logic or something to handle statements better? Losing the return value kinda sucks.

I think that is a clear limitation with the current approach entirely.

The return values still work out fine for standard last statement semantics - it's the more complex control flows that this only seems to apply to where console.log can still suffice as a workflow for these.

Agreed its sub optimal, but I don't think it would be worth blocking this entire feature on that, especially when we have the better improvements here in the works.

[guybedford]

@devsnek I've just landed #39154 to address your first concern here. I won't be able to address the other concern though personally here due to the complexity of creating a custom runtime transform to handle return locations, although I agree with the future directions of upstreaming the debugger API.

If you are still against this PR can you make it explicit again on this issue given this PR is to the latest rebases etc.

Ping for further reviewers as well.

[aduh95]

Related: #39259.

[devsnek]

@guybedford ^ this is the sort of thing i mean. i won't block this but i think it would be an overall bad direction to unflag this.

[guybedford]

@devsnek agreed that isn't great at all. Ideally these transforms should be comprehensively working for basic execution semantics down to only not supporting eg const or perfect return values if we do want to unflag. But having let vars as globals should be fixed to unflag here...

[guybedford]

Closing as a duplicate of #34733.