Closed Bug 1473463 Opened Last year Closed 4 months ago

Evaluate worklet scripts as modules

Categories

(Core :: DOM: Core & HTML, enhancement, P3)

enhancement

Tracking

()

RESOLVED FIXED
mozilla70
Tracking Status
firefox63 --- wontfix
firefox70 --- fixed

People

(Reporter: karlt, Assigned: karlt)

References

(Blocks 2 open bugs)

Details

Attachments

(3 files)

Andrea said in https://bugzilla.mozilla.org/show_bug.cgi?id=1472324#c1
'there is still a bigger issue to solve related to the importing of scripts as
module in workers (and worklet). See bug 1247687. Currently we support script
modules only on main-thread via <script type="module">. In order to support
script modules in workers/worklet, we must unify WorkletFetchHandler,
workerinternals::Load/LoadMainScript and mozilla::dom::ScriptLoader.'

Ben said in https://bugzilla.mozilla.org/show_bug.cgi?id=1442776#c1
"Would it not be possible to make worklets and workers completely separate?
Decoupling them seems like it would be nice for maintenance since they are
going to be fairly different."
That probably means that any shared code should be in or near ScriptLoader,
rather than in dom/worklers.

Jon said in https://bugzilla.mozilla.org/show_bug.cgi?id=1290021#c34
"It would be really great if we could reuse some of the code in the script
loader (and would be useful for other efforts, e.g. bug 1308512).  This is not
trivial unfortunately.  I think it would require factoring out the module
loader into a separate class with a caller-provided a fetch interface passed
in."
This would be bug 1311726.

Jon said in https://bugzilla.mozilla.org/show_bug.cgi?id=1290021#c40
"As long as the module doesn't import anything you can load it by calling
JS::CompileModule to get a module record object, and then calling
JS::ModuleDeclarationInstantiation and then JS::ModuleEvaluation on that."

Houdini WG said in
https://github.com/w3c/css-houdini-drafts/issues/506#issuecomment-343276228
"RESOLVED: dynamic import() in worklets is a no-op that returns a rejected
promise (or equivalent after passing thru JS people)"

Web platform tests expect static import to work.
https://github.com/w3c/css-houdini-drafts/issues/506#issuecomment-342796390
Priority: -- → P3
Component: DOM → DOM: Core & HTML

One thing that will likely fall into place with this work is that the Worklet.addModule() promise should only be rejected when "fetch a worklet script" (which includes ParseModule()) fails. Errors during "run a module script" should not cause the promise to be rejected (but do in current code because JS::Evaluate() is used to parse).
https://drafts.css-houdini.org/worklets/#fetch-and-invoke-a-worklet-script

https://github.com/w3c/css-houdini-drafts/issues/407 questions whether that is sane behavior.

Blocks: 1572644

The issue in comment 1 is getting in the way of writing tests for exception reporting, and so I'll address that with a simple module script implementation without import support, and track implementation of fetching a complete module script graph in bug 1572644.

Assignee: nobody → karlt

The separation of parse and evaluate steps permits implementation of behavior
on unhandled exception during evaluation that is different from that on
parsing error.

Depends on D41338

No longer depends on: 1311726
Attachment #9084234 - Attachment description: Bug 1473463 evaluate worklet scripts as modules r?baku → Bug 1473463 evaluate worklet scripts as modules without support for nested import r?baku
Pushed by ktomlinson@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/f532972750b5
use globalThis in worklet tests because |this| is undefined in toplevel module script r=baku
https://hg.mozilla.org/integration/autoland/rev/e67d048b2d11
evaluate worklet scripts as modules without support for nested import r=baku
https://hg.mozilla.org/integration/autoland/rev/ba392537f7a9
remove unnecessary JSAutoRealm as realm is set by AutoEntryScript r=baku
You need to log in before you can comment on or make changes to this bug.