bootstrap: --frozen-intrinsics override workaround #28254
Conversation
|
Sadly, an error occurred when I tried to trigger a build. :( |
There was a problem hiding this comment.
I think there's a more recent version of beMutable around somewhere. The name beMutable is my fault. It is terrible, since it does not make any object mutable. I think the more recent version, if I can find it, had a marginally better name but not a good one. Suggestions appreciated.
Doing this repair on all prototypes I suspect is overkill. Salesforce does it only for IIRC 5: Object.prototype, Array.prototype, Error.prototype, Function.prototype, and RegExp.prototype.
For the current state without this PR, can we gather statistics of where we're actually running into the override mistake on a corpus of real code?
Attn @jfparadis @michaelfig
|
Certainly we could reduce the application of the fix and then just apply it when we hit bugs (like we did with console in #27663), my concern with that is just that the bugs may be highly unexpected and difficult to track. While we could automate that process by running this flag against the CI smoke test libraries and see how far it gets, fixing as necessary, there will always be edge cases that will be missed. I've been trying to think of this in terms of something we could imagine being lowered into v8 and a spec at some point, where the generic approach feels like it is probably the better way to ensure consistency? What would be the concerns of applying it to all intrinsic prototypes? Will put some thought to naming too. |
|
I agree there's no clear advantage to being stingy on applying this trick. Let's try it your way and see if we find any problems. |
guybedford
force-pushed
the
frozen-intrinsics-define
branch
2 times, most recently
from
e4b11c9
to
cb82de1
Compare
|
I've changed the naming to |
guybedford
force-pushed
the
frozen-intrinsics-define
branch
from
2acef16
to
537914a
Compare
guybedford
force-pushed
the
frozen-intrinsics-define
branch
from
537914a
to
e94164d
Compare
|
Landed in 554ffa3. |
PR-URL: #28254 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: James M Snell <jasnell@gmail.com>
PR-URL: #28254 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: James M Snell <jasnell@gmail.com>
So it turns out I've been doing it wrong this whole time and SES already has a solution to the "override problem".
This adopts the approach taken in https://github.com/tc39/proposal-ses/blob/e5271cc42a257a05dcae2fd94713ed2f46c08620/shim/src/mutable.js, which is to turn all intrinsic prototype value properties into getters and setters, with the setter updating the object as if it were still a value property in the override case.
This then gets all those edge cases that were previously documented with work-arounds to work out correctly, meaning that most third-party code becomes compatible with this flag. I just tested this on a large Node.js application and everything worked fine! I've also re-enabled the freezing of error prototypes and console now that these are supported.
Thanks @erights for the pointer on this one.
Checklist
make -j4 test(UNIX), orvcbuild test(Windows) passes