-
Notifications
You must be signed in to change notification settings - Fork 30.5k
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
deprecate --use_strict #6429
Comments
cc @nodejs/v8 |
Arguments are passed verbatim to v8, I'm not sure we're keen on altering that.. |
I'm -1 on interfering with (or documenting) command-line options provided by v8. |
-1, this flag is useful |
@vkurchatkin Mind to elaborate on that? |
How is it useful? The flag is not the same as |
As recently as #6377, we rejected documenting V8 flags. This goes even further beyond that. I think it would be simplest for people to just not use it. |
The reasons for rejecting documentation is given as "… those are not under our control to change but as soon as we document it they become our responsibility to maintain". This is different - this is ensuring with certainty that no node users could ever depend on the v8 flag, which reduces the maintenance needed from the node core team (aside from maintaining a blacklist, for example) |
In other words, if |
Don't want to state the obvious, but it's useful when you want to run all javascript in strict mode. I think it's well understood that strict mode is not the default mode only because it's not backwards compatible. |
@vkurchatkin yes but |
I agree that most people don't need it, but that doesn't mean that we should remove it. For example I use v8 on android to run some js in strict mode and use node to test this code on desktop. There are no dependencies, so I use --use_strict |
@ljharb There are 389 options in Also, what evidence is there that this flag is actually problematic or harmful in the ecosystem? |
@trevnorris People sometimes report issues like «your module is broken and doesn't work … I used Still, I don't think that blacklisting this would do any good — some people might need it, as @vkurchatkin noted, and there are a lot more flags that affect the behaviour in a unsafe way, and not only the ones that @trevnorris noted. E.g. IMO, we shouldn't blacklist anything, but we could print a warning if certain flags are detected (without modifying the flags, though). But such warning should cover much more than just Btw, there is even such a thing as https://github.com/isaacs/use-strict. |
I'd be in favor of warning against |
@Fishrock123 And |
Note as well that if node truly wants VM neutrality, blindly passing through engine flags isn't going to be sustainable - so node may soon have to maintain an exclusion list/inclusion list of flags anyways. |
@Fishrock123 ... docs warning or runtime warning? |
@ljharb I don't see maintaining that type of exclusion list as an option. It would be impossible to debug and tune for individual VMs if all options to do so were removed. For example, while the |
Could you elaborate why it wouldn't be? I don't see any problem here. (Also, developing our own vm flags is pretty silly and out of scope for sure.) |
@jasnell presumably runtime, might have to be in c++ though I think, since this happens before the VM boots up. |
@ljharb this is one of the reasons why true VM neutrality is very unlikely. Each VM has it's own flags for tuning critical subsystems (e.g. GC), which will be very hard to reconcile |
I don't see a reason we have to be VM neutral on flags when we can just keep passing flags to the VM that is being used. |
@Fishrock123 ... yep, I've had it on my todo to see if we can hook into process warning easily from native but a simple message printed to stderr on startup works for me.... Perhaps it would be worthwhile printing the warning when any VM specific flag is used? If |
@jasnell I don't think this justifies warnings on debug flags, e.g. |
FTR, I disagree completely with blacklisting or warning. I'm an adult, I don't need a computer program telling me what I should or shouldn't do. It's paternalistic. |
@bnoordhuis, this isn't something like "don't do this" kind of warning, I was thinking of something that is more like a friendly reminder, e.g. "Notice: using flagged harmony features might not work as expected and might even crash the process". |
About forwarding flags: Chromium embeds V8 by forwarding flags through a special option, |
Users should decide if they want strict mode or not. I initially added it because Node had it, but it turns out it's a V8 flag really, not a Node flag, and it's not really recommended to use. Refs: nodejs/node#6429
Users should decide if they want strict mode or not. I initially added it because Node had it, but it turns out it's a V8 flag really, not a Node flag, and it's not really recommended to use. Refs: nodejs/node#6429
I'm closing this issue, it's been idle for some time, but reopen if you disagree. I believe the dominant sentiment is that we won't be adopting this proposal. |
I'd like to reopen the discussion from nodejs/node-v0.x-archive#7479 if possible about somehow deprecating/removing this flag.
Whether the v8 team is willing to remove or rename it, or not (which we should probably re-investigate), I would like node to find a way to have it be removed or renamed.
The text was updated successfully, but these errors were encountered: