src: add no-op for --harmony-proxies flag

[evanlucas]

Checklist

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

Affected core subsystem(s)

src

Description of change

The upgrade from V8 5.0 to V8 5.1 removed the --harmony-proxies flag.
This restores them as no-ops to prevent breakage.

Fixes: #8388

[mscdex]

This seems rather ugly. Hasn't this happened before and we just accepted it as V8 ships features?

[Fishrock123]

@mscdex I would be surprised if we did in a non-major release that was backed by the current stability policies.

[evanlucas]

@mscdex I agree. I guess we could add the flag back in the flag-definitions?

[jasnell]

Yeah, not a fan of this approach as it sets a bad precedent -- I don't want to be forced to add a new flag in node.cc every time v8 decides to pull it.

[silverwind]

Hmm, how is someone ever going to notice that they are providing a nonexistant flag if we just noop it? Would it be possible to print a warning instead?

[silverwind]

Maybe we are blowing this out of proportion, though. V8 flags always were unstable, and I'm leaning to just documenting that fact.

[addaleax]

There were quite a few flags removed in v6.5:

• --use_strong
• --strong_mode
• --strong_this
• --legacy_const
• --harmony_modules
• --harmony_default_parameters
• --harmony_destructuring_assignment
• --harmony_destructuring_bind
• --harmony_tostring
• --harmony_regexps
• --harmony_proxies
• --harmony_reflect
• --array_bounds_checks_hoisting
• --concurrent_osr
• --trace_wasm_ast
• --dump_asmjs_wasm
• --asmjs_wasm_dumpfile
• --debug_eval_readonly_locals
• --log_snapshot_positions

[jasnell]

I know everyone is likely sick of warnings ;-) ... but... one solution would be to no-op these and print a warning at startup when they are used... just to make it visible that they're no longer necessary/useful.

[evanlucas]

@addaleax thanks, I didn't realize that. I guess we could add them back to flag definitions (Not sure we would even want to with this many though)?

[mscdex]

If we're going to go down the path of smoothing over V8 changes, wouldn't it be better to float a patch on top of V8 instead of adding them to node core? They are V8 options, not node options.

The other option is to back out V8 5.1 out of v6.x.

I'm -1 on this in general though.

[Fishrock123]

Floating a patch on V8 may be easier, since it will take advantage of the options parser.

[MylesBorins]

+1 to floating a patch on V8.

We should likely come up with a policy for updating V8 in a release stream, personally I think that including no-ops for removed flags is a fairly low effort and reasonable thing to offer individuals.

Alternatively we can head in the direction that we do not support these flags, but it seems like the amount of work to do is fairly minimal compared to the number of people that will have to potentially make changes on already deployed applications to run with the upgrade.

[evanlucas]

I'll update the PR with an attempt at that tomorrow. Thanks!

[bnoordhuis]

+1 to floating a patch on V8.

Strongly -1.

[thetutlage]

Being the author of a framework, it is a nightmare for me to run tests on Travis(or any CI). I have to write Node.js scripts checking the node version and then running tests with dynamic harmony_proxies flag.

[targos]

@thetutlage You shouldn't have to do this. The Proxy constructor is available without a flag since v6.0.0.

[thetutlage]

@targos I agree to that. My framework has support from Node.js 4.0 - Latest version. According to semver switch from 5.0 to 6.0 is a major release, which allows Node.js to have breaking changes.

But taking a practical example, where frameworks generally don't drop support for older versions quickly, it is kind of pain to write work around.

It would have been nice, if Node.js v6.0 would have started giving warnings, which gives us some time to act upon it.

[cjihrig]

I'm -1 on this. In the past we've said that we don't touch the V8 flags (such as turning off the strict mode flag).

[jbergstroem]

-1. My vote would go on improving documentation about flags like these be treated as experimental and not relied upon.

[evanlucas]

Closing due to the number of -1's. I think @ofrobots is going to open one that isn't so ugly. Thanks! :]

[Fishrock123]

I am not super worried about --harmony flags, but if others could disappear during a an exiting major release stream that seems problematic.

[Fishrock123]

On second thought, I still strongly think we should do this. (Probably on V8, it's not like other stuff wasn't floated.)

What is @ofrobots's plan?

I'm -1 on this. In the past we've said that we don't touch the V8 flags (such as turning off the strict mode flag).

That's not the same as your application no longer starting in a minor upgrade, no matter how "easy" the fix.

Edit: I have some leeway for experimental flags, similar to async_wrap, but see my comment above.

[ofrobots]

Sorry, I didn't know about this thread.

Generally I would agree with the argument that V8 flags are not supported by Node and users should not have an expectation of them continuing to work. In the case of the shipping harmony flags (nops) that were removed, it is going to be trivial one liners to add them back in (as nops), specially given that the harmony flags do get some use in the ecosystem. I am strongly against doing anything with any of the other removed flags.

[jasnell]

I would definitely be in favor of making these noops when they disappear from v8 in patch or minor releases, and removing them entirely in majors. I would prefer that a deprecation warning be printed but I'm ok with simply ignoring them.

[yosuke-furukawa]

I don't understand the discussion point, but IMHO, experimental features like --harmony flags, we need not to follow semver. we would be better to clarify our support policy about experimental features.

[addaleax]

we need not to follow semver

That might be a different the question, maybe we don’t need to, yeah. But still, we clearly can do something on our part that comes at relatively little cost (and where there are clearly volunteers) and might save a much much larger group of people quite a bit of work.

[ofrobots]

My attempt: #8445