[worklets] dynamic import()

[annevk]

I thought we already had an issue for this somewhere. If worklets cannot have fetch(), I don't think it makes sense for them to have dynamic import(). That would cause the same kind of execution problems.

cc @domenic @nhiroki @bfgeek

[annevk]

See whatwg/fetch#527 (comment) for issues if we decide to allow it somehow.

[bfgeek]

I agree that it makes sense we shouldn't have import().

[bfgeek]

I'm not sure what the behaviour should be, if import() is just hidden, or it throws. :/

[surma]

If we are disallowing it, I’d say hide it. A function whose only purpose is to throw seems odd.

[domenic]

It's not a function; it's syntax. It can't be removed without language changes. I'm not even sure what hiding it would mean.

[bfgeek]

Oh didn't realize that, makes sense.

[annevk]

@domenic I assume we can make it reject the promise it returns? (Depending on what data gets passed to Fetch, we could even return a network error there for all fetches made by worklets. Not sure that's ideal however as it's unlikely to be implemented that way.)

[domenic]

Yeah, that's doable.

[nhiroki]

Filed a tracking issue in crbug.

[domenic]

To be clear, I don't think we should reject import() in worklets, per my position from the earlier thread on the same topic. I think all forms of importing code should work in worklets, including lazily importing. I was only commenting on the technical feasibility here.

[annevk]

I wonder how worklets work with the opaque origin setup if their modules can have non-dynamic imports. Did you review that @domenic? Or maybe the opaque origin is assigned after all dependencies are fetched?

The main reason to block dynamic import() is the same reason we block fetch(); to make it clear worklets are not supposed to have that kind of code.

[nhiroki]

I wonder how worklets work with the opaque origin setup if their modules can have non-dynamic imports. Did you review that @domenic? Or maybe the opaque origin is assigned after all dependencies are fetched?

IIUC, non-dynamic imports are handled as sub-resources of the owner document like addModule(), so the opaque origin is not a problem. Chromium implemented this behavior and there are WPT tests for the case.

[annevk]

Thanks. But the referrer would be the top-level module script's (base) URL? So yeah, if we want dynamic import() to work we'd need some justification as to why that's okay and fetch() is not and then figure out how it should actually work as per the comment following OP.

[nhiroki]

But the referrer would be the top-level module script's (base) URL?

Yes. It's the same with plain ES6 modules. I'm now adding WPT tests for the referrer of descendant scripts (crbug).

[bfgeek]

Part of the justification that it shouldn't work for me at least, is that worklet global scopes are allowed to be transient, and when they are created again they should contain all the same code as the other WGS. Allowing the dynamic import here makes it a little too easy for them to diverge.

We won't be able to remove all non-deterministic apis, but this one seems easy to reason about?

[css-meeting-bot]

The Working Group just discussed dynamic import(), and agreed to the following resolutions:

• RESOLVED: dynamic import() in worklets is a no-op that returns a rejected promise (or equivalent after passing thru JS people)

The full IRC log of that discussion

<TabAtkins> Topic: dynamic import()
<TabAtkins> github: https://github.com//issues/506
<TabAtkins> iank_: ES6 modules are a thing
<TabAtkins> iank_: worklets are built on them
<TabAtkins> iank_: WE have a dynamic import() now
<TabAtkins> iank_: you can say import(url), returns a promise
<TabAtkins> iank_: Do we want to allwo in worklets?
<TabAtkins> TabAtkins: And we don't allow fetch(), so...
<TabAtkins> Rossen: Motivation?
<TabAtkins> iank_: Consistency with other global scopes.
<TabAtkins> smfr: Can't do anything async, right? The paint has be to sync.
<TabAtkins> iank_: Right. It would just be a communication channel to the server.
<TabAtkins> dbaron: Woudl this expose whether you're recycling the global?
<TabAtkins> iank_: Potentially. It wouldn't be intercepted by a SW.
<TabAtkins> smfr: And it would mean your context is running code at a weird time.
<TabAtkins> iank_: So room opinion seems to be to throw?
<TabAtkins> TabAtkins: If we don't allow any other network communication...
<TabAtkins> Rossen: Yeah, with all the worklets, just makes more sense.
<TabAtkins> iank_: So the proposed resolution is that dynamic import becomes a no-op that returns a rejected promise.
<TabAtkins> RESOLVED: dynamic import() in worklets is a no-op that returns a rejected promise (or equivalent after passing thru JS people)

[domenic]

I don't think we should be piecemeal disabling parts of the JS language. It should be principled, e.g. adding a switch that disables dynamic import and Math.random() and Date.now()/new Date()/etc. at the same time.

[annevk]

It is principled, no network APIs.

[domenic]

Then static import should be disallowed.

[annevk]

Why? Those are all resolved before execution.

[domenic]

They're still network APIs, so it seems the line is not very principled?

[bfgeek]

Anne is right. This keeps inline with "no communication to the outside world" once you've added the scripts, i.e. no networking apis.

[nhiroki]

What error code does import() return on WorkletGlobalScope? NotSupportedError?

[nhiroki]

If there is no objection, I'm going to make import() on WorkletGlobalScope reject the promise with NotAllowedError (CL).

[annevk]

Apologies for not replying sooner. I think TypeError would be better for a JavaScript language construct.

[nhiroki]

@annevk Thank you! TypeError sounds good. I didn't know NotSupportedError is not a native error type defined in the ECMAScript spec. I'll update the CL.