Skip to content

Conversation

@anonrig
Copy link
Member

@anonrig anonrig commented Dec 22, 2025

This is important for passing several web-platform tests related to streams and fetch.

We use null prototype to prevent Object.prototype.then patches from intercepting internal promise operations (required for WPT compliance).

@anonrig anonrig requested a review from jasnell December 22, 2025 20:42
@anonrig anonrig requested review from a team as code owners December 22, 2025 20:42
@jasnell
Copy link
Collaborator

jasnell commented Dec 23, 2025

An alternative way of doing this that might be better, is to add the following when the opaqueTemplate is created:

auto protoTemplate = v8::FunctionTemplate::New(ptr, &throwIllegalConstructor);
protoTemplate->RemovePrototype();
opaqueTemplate->SetPrototypeProviderTemplate(protoTemplate);

This would avoid the added branch and after-creation prototype mutation in the hotpath and allows the wpt's to pass. I haven't benchmarked to see if it actually is faster tho but it's worth an experiment.

@anonrig
Copy link
Member Author

anonrig commented Dec 23, 2025

@jasnell I believe you're right and I think it is actually faster than the current state.

@anonrig anonrig force-pushed the yagiz/use-null-prototype-for-opaque-wrappers branch from 8487bfc to e9e8876 Compare December 23, 2025 21:38
@anonrig anonrig force-pushed the yagiz/use-null-prototype-for-opaque-wrappers branch from e9e8876 to 23ac30e Compare December 23, 2025 21:39
@anonrig anonrig requested a review from jasnell December 23, 2025 22:28
Copy link
Collaborator

@jasnell jasnell left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. I do think it might be beneficial for either the PR or commit message to explain a bit about WHY this fixes the tests as it's non-obvious, but that's not necessarily blocking.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants