-
Notifications
You must be signed in to change notification settings - Fork 2.7k
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
Should we delete or otherwise neuter globalThis in worklets? #6059
Comments
Would it be possible to freeze |
I'm not very familiar with Worklets. Do different runs of a worklet get a different |
That's also a very interesting option. I wonder if it's been considered in the past? @bfgeek?
They generally share the same one. The issue is that the runs will be done in an implementation-defined way. E.g., one browser might spin up 2 WorkletGlobalScopes and run all CSS custom paint code in those two in an alternating, deterministic fashion; another might spin up 5 and choose at random; another, very inefficient, browser might destroy and recreate WorkletGlobalScopes every time it needs to do a custom paint operation. Because of this by-design interop issue, the spec is trying to nudge web developers to write their code to be more idempotent and less stateful, to avoid in-practice interop issues. See https://drafts.css-houdini.org/worklets/#code-idempotency and https://drafts.css-houdini.org/worklets/#speculative-evaluation for a bit more background on that. So concretely, it's bad if authors write worklet code that stores state on the global object, because it might stick around in one browser between calls into the worklet code, whereas in another browser it might not stick around. |
Thanks for the very helpful background @domenic! For the intention of discouraging easy-to-reach-for language features that'll make code accidentally non-idempotent, I like @Yay295's suggestion to shallow freeze the global object the most. A note that one may be tempted to take this approach further and shallow freeze built-in prototypes as well, to close off that avenue of non-idempotence. But this is not possible due to the "override mistake", whereby a frozen property on the prototype prevents assignment of a same-named property on instances, freezing built-in prototypes is highly likely not web compatible. Also I'd probably rule out option 3 in #6059 (comment), because they're too involved for the surface they're trying to plug. The general problem of "make global state impossible to store" is very hard in general and to make future proof, and the extra spec and implementation contortions there seem not worth the effort. |
cc @padenot |
No, this is required for |
@padenot I understand that state is required for audio worklet, but is |
It's useful if one wants to share data between different instances of |
Alright. I find it kind of strange that AudioWorkletGlobalScope doesn't define |
Well, I think it's still worth considering for non-audio worklets. And I think you make a good point that audio worklets should probably expose |
cc @hoch This sounds like something we want to do yes. |
+1 on exposing |
I wonder if @tabatkins has opinions here or knows someone who can give input from the perspective of the CSS-related worklets. I'm also not sure how I can get <script type=module>
console.log(eval("this")); // undefined
</script> |
@annevk needs some indirection, eval lives in a weird limbo between regular function and syntax (step 6 at that link). |
Okay, so I think this primarily hinges on what we want for CSS-related worklets and to what extent they're still malleable due to lack of multiple implementations. If they are malleable, freezing seems interesting given the discussion above. What's left at that point is whether |
The worklets spec currently claims
In the process of moving this to HTML in #6056, I noticed that this is no longer a true statement. Since the Worklets spec was written, the JavaScript specification has introduced the
globalThis
global, which points to the global this value/global object.I see several options here:
Do nothing, and let
globalThis
continue to live and point to theWorkletGlobalScope
object. The main justification for this is that the purpose ofglobalThis
was to provide cross-environment uniformity, and we shouldn't counteract that. This would involve rephrasing that section of the spec to admit that you can store global state if you want to; the browser won't stop you.Delete the
globalThis
property onWorkletGlobalScope
creation, like we currently do forSharedArrayBuffer
in some cases. This gets us back to the status quo when worklets were originally specced, at the cost of going against the spirit of the JS specification. (But not the letter; hosts are allowed to change global properties, just like JS code can.)Say that the "global this value" in worklets is
undefined
, even though the global object is aWorkletGlobalScope
. This would have the following more-subtle effects:eval("this")
, which evalsthis
in sloppy mode global scope, would returnundefined
. I.e., this would plug the current way in which code can escape the strict/module-level global scope worklets are usually confined to. That means the global is really super-censored, even more than it ever has been.hasOwnProperty('globalThis')
would still returntrue
(unlike with option (2)).This would probably require ES spec updates, as currently the ES spec requires the value to be an Object. At a quick skim, however, it seems like nothing in the ES spec would break if we relaxed this constraint.
Probably this would be harder to implement than (2).
/cc @syg for ES side considerations; @nhiroki for Chrome worklet stuff; @bfgeek as worklets editor.
The text was updated successfully, but these errors were encountered: