WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
164860
JS Modules in Workers
https://bugs.webkit.org/show_bug.cgi?id=164860
Summary
JS Modules in Workers
Yusuke Suzuki
Reported
2016-11-17 01:18:38 PST
...
Attachments
Patch
(27.74 KB, patch)
2020-06-19 16:47 PDT
,
Yusuke Suzuki
no flags
Details
Formatted Diff
Diff
Patch
(28.34 KB, patch)
2021-02-07 23:33 PST
,
Yusuke Suzuki
no flags
Details
Formatted Diff
Diff
Patch
(28.32 KB, patch)
2021-02-08 01:26 PST
,
Yusuke Suzuki
ews-feeder
: commit-queue-
Details
Formatted Diff
Diff
Patch
(29.04 KB, patch)
2021-02-08 01:52 PST
,
Yusuke Suzuki
no flags
Details
Formatted Diff
Diff
Patch
(66.94 KB, patch)
2021-02-09 03:04 PST
,
Yusuke Suzuki
no flags
Details
Formatted Diff
Diff
Patch
(97.78 KB, patch)
2021-02-09 20:26 PST
,
Yusuke Suzuki
ews-feeder
: commit-queue-
Details
Formatted Diff
Diff
Patch
(102.99 KB, patch)
2021-02-10 00:29 PST
,
Yusuke Suzuki
ews-feeder
: commit-queue-
Details
Formatted Diff
Diff
Patch
(111.39 KB, patch)
2021-02-10 00:39 PST
,
Yusuke Suzuki
no flags
Details
Formatted Diff
Diff
Patch
(111.46 KB, patch)
2021-02-10 13:40 PST
,
Yusuke Suzuki
no flags
Details
Formatted Diff
Diff
Patch
(140.58 KB, patch)
2021-02-10 16:46 PST
,
Yusuke Suzuki
no flags
Details
Formatted Diff
Diff
Patch
(145.20 KB, patch)
2021-02-10 18:11 PST
,
Yusuke Suzuki
no flags
Details
Formatted Diff
Diff
Patch
(171.03 KB, patch)
2021-02-11 11:24 PST
,
Yusuke Suzuki
no flags
Details
Formatted Diff
Diff
Patch
(224.68 KB, patch)
2021-02-11 14:55 PST
,
Yusuke Suzuki
ews-feeder
: commit-queue-
Details
Formatted Diff
Diff
Patch
(224.52 KB, patch)
2021-02-11 15:13 PST
,
Yusuke Suzuki
no flags
Details
Formatted Diff
Diff
Patch
(230.10 KB, patch)
2021-02-11 20:18 PST
,
Yusuke Suzuki
no flags
Details
Formatted Diff
Diff
Patch
(230.11 KB, patch)
2021-02-11 20:26 PST
,
Yusuke Suzuki
no flags
Details
Formatted Diff
Diff
Patch
(230.21 KB, patch)
2021-02-11 22:52 PST
,
Yusuke Suzuki
no flags
Details
Formatted Diff
Diff
Patch
(232.99 KB, patch)
2021-02-12 01:12 PST
,
Yusuke Suzuki
no flags
Details
Formatted Diff
Diff
Patch
(232.99 KB, patch)
2021-02-12 22:57 PST
,
Yusuke Suzuki
no flags
Details
Formatted Diff
Diff
Patch
(233.37 KB, patch)
2021-02-17 01:58 PST
,
Yusuke Suzuki
no flags
Details
Formatted Diff
Diff
Patch
(241.54 KB, patch)
2021-02-18 23:02 PST
,
Yusuke Suzuki
no flags
Details
Formatted Diff
Diff
Patch
(244.69 KB, patch)
2021-02-18 23:36 PST
,
Yusuke Suzuki
no flags
Details
Formatted Diff
Diff
Patch
(245.29 KB, patch)
2021-02-19 11:26 PST
,
Yusuke Suzuki
saam
: review+
Details
Formatted Diff
Diff
Advanced use case
(798.71 KB, image/png)
2021-04-24 02:09 PDT
,
Tobias Uhlig
no flags
Details
Show Obsolete
(22)
View All
Add attachment
proposed patch, testcase, etc.
Saleh Abdel Motaal
Comment 1
2018-07-07 05:59:18 PDT
It seems that support `new Worker(…, {type: "module"})` is limited or missing. In a project that uses only pure ES modules served using a very basic express server (ie express.static) over
http://127.0.0.1:8080
or
http://localhost:8080
a worker module with static import statements: ``` import {…} from '…' ``` Throws: SyntaxError: Unexpected token '{'. import call expects exactly one argument. It should be noted that those static import statements are synthesized using rollup and the resulting files sometimes include one or more variable declarations before the occurrence of the import statements. The same project does not throw in Chrome.
Guy Bedford
Comment 2
2019-09-11 11:47:33 PDT
Note that supporting dynamic `import()` in Web Workers can also be a first start along these lines.
Tobias Uhlig
Comment 3
2019-11-03 12:02:36 PST
Just stumbled on this ticket and am quite surprised that it did not get way more comments yet. I am crafting a web workers driven ES8 framework, so module support for workers (including dynamic imports) is one of the key features lacking in Safari. In Chrome this works already for close to a year using the experimental web plattform features and now got the Label Target-80. Without the type="module" support, applications have to use build tools like Webpack, which is slowing the web development speed down by a long shot! Please be aware, that this ticket should also include the module type for shared & service workers. I recommend to make it a high priority epic. Thanks & best regards Tobias
Tobias Uhlig
Comment 4
2019-12-20 13:06:22 PST
A quick update on the current status would be appreciated. The new framework is online now and already at 227 github stars (December 20):
https://github.com/neomjs/neo
Here are the online examples, which according to the specs should work in every modern browser:
https://neomjs.github.io/pages/
It makes me a bit sad to see Safari falling behind on an such important topic.
https://html.spec.whatwg.org/multipage/workers.html#module-worker-example
Hoping you will catch up! Best regards & happy holiddays, Tobias
Radar WebKit Bug Importer
Comment 5
2020-06-14 19:44:14 PDT
<
rdar://problem/64348230
>
Yusuke Suzuki
Comment 6
2020-06-19 16:47:17 PDT
Created
attachment 402347
[details]
Patch WIP
Yusuke Suzuki
Comment 7
2021-02-07 23:33:13 PST
Created
attachment 419557
[details]
Patch
Yusuke Suzuki
Comment 8
2021-02-08 01:26:54 PST
Created
attachment 419562
[details]
Patch
Yusuke Suzuki
Comment 9
2021-02-08 01:52:58 PST
Created
attachment 419565
[details]
Patch
Yusuke Suzuki
Comment 10
2021-02-09 03:04:39 PST
Created
attachment 419694
[details]
Patch
Yusuke Suzuki
Comment 11
2021-02-09 20:26:57 PST
Created
attachment 419807
[details]
Patch
Yusuke Suzuki
Comment 12
2021-02-10 00:29:20 PST
Created
attachment 419819
[details]
Patch
Yusuke Suzuki
Comment 13
2021-02-10 00:39:08 PST
Created
attachment 419821
[details]
Patch
Yusuke Suzuki
Comment 14
2021-02-10 13:40:32 PST
Created
attachment 419895
[details]
Patch
Yusuke Suzuki
Comment 15
2021-02-10 16:46:59 PST
Created
attachment 419922
[details]
Patch
Yusuke Suzuki
Comment 16
2021-02-10 18:11:32 PST
Created
attachment 419935
[details]
Patch
Yusuke Suzuki
Comment 17
2021-02-11 11:24:59 PST
Created
attachment 420011
[details]
Patch
EWS Watchlist
Comment 18
2021-02-11 11:26:03 PST
This patch modifies the imported WPT tests. Please ensure that any changes on the tests (not coming from a WPT import) are exported to WPT. Please see
https://trac.webkit.org/wiki/WPTExportProcess
Yusuke Suzuki
Comment 19
2021-02-11 14:55:57 PST
Created
attachment 420047
[details]
Patch
Yusuke Suzuki
Comment 20
2021-02-11 15:13:12 PST
Created
attachment 420050
[details]
Patch
Yusuke Suzuki
Comment 21
2021-02-11 20:18:34 PST
Created
attachment 420084
[details]
Patch
Yusuke Suzuki
Comment 22
2021-02-11 20:26:44 PST
Created
attachment 420086
[details]
Patch
Yusuke Suzuki
Comment 23
2021-02-11 22:52:30 PST
Created
attachment 420100
[details]
Patch
Yusuke Suzuki
Comment 24
2021-02-12 01:12:45 PST
Created
attachment 420102
[details]
Patch
Yusuke Suzuki
Comment 25
2021-02-12 22:57:40 PST
Created
attachment 420214
[details]
Patch
Yusuke Suzuki
Comment 26
2021-02-17 01:58:07 PST
Created
attachment 420618
[details]
Patch
Yusuke Suzuki
Comment 27
2021-02-18 23:02:39 PST
Created
attachment 420925
[details]
Patch
Yusuke Suzuki
Comment 28
2021-02-18 23:36:33 PST
Created
attachment 420927
[details]
Patch
Yusuke Suzuki
Comment 29
2021-02-19 11:26:31 PST
Created
attachment 420996
[details]
Patch
Saam Barati
Comment 30
2021-02-19 19:37:10 PST
Comment on
attachment 420996
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=420996&action=review
r=me Do we want some more tests here? Especially for various error cases, and various complex programs and such?
> Source/WebCore/bindings/js/JSDOMExceptionHandling.cpp:172 > + // FIXME: All callers to createDOMException need to pass in the correct global object. > + // For now, we're going to assume the lexicalGlobalObject. Which is wrong in cases like this: > + // frames[0].document.createElement(null, null); // throws an exception which should have the subframe's prototypes.
maybe file a bug for this?
> Source/WebCore/bindings/js/ScriptModuleLoader.cpp:332 > +void ScriptModuleLoader::notifyFinished(ModuleScriptLoader& moduleScriptLoader, URL&& sourceURL, Ref<DeferredPromise> promise)
nit: why not just take URL here (callers can still move into the parameter)? That way we know this function takes ownership of URL. I don't think this sourceURL parameter is actually moved anywhere here, or at least there are early return paths that don't take ownership over it.
> Source/WebCore/bindings/js/ScriptModuleLoader.cpp:404 > + rejectToPropagateNetworkError(promise.get(), ModuleFetchFailureKind::WasCanceled, "Importing a module script is canceled."_s);
nit: I wonder if this error should be more like "Loading a module script was cancelled" or "Load for importing a module script was canceled"
> Source/WebCore/workers/WorkerGlobalScope.cpp:282 > + return Exception { TypeError, "importScripts cannot be used if woerker type is \"module\""_s };
"woerker" => "worker"
> Source/WebCore/workers/WorkerOrWorkletScriptController.cpp:364 > + // Overwrite the detailed error with a generic error.
nit: not sure this comment is adding anything
> Source/WebCore/workers/WorkerOrWorkletScriptController.cpp:425 > + // Overwrite the detailed error with a generic error.
ditto I wonder if we can share this code w/ above that seems to do a similar thing.
Yusuke Suzuki
Comment 31
2021-02-20 11:47:59 PST
Comment on
attachment 420996
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=420996&action=review
I think all error things are already tested. So, it is OK :)
>> Source/WebCore/bindings/js/JSDOMExceptionHandling.cpp:172 >> + // frames[0].document.createElement(null, null); // throws an exception which should have the subframe's prototypes. > > maybe file a bug for this?
Filed
https://bugs.webkit.org/show_bug.cgi?id=222229
>> Source/WebCore/bindings/js/ScriptModuleLoader.cpp:332 >> +void ScriptModuleLoader::notifyFinished(ModuleScriptLoader& moduleScriptLoader, URL&& sourceURL, Ref<DeferredPromise> promise) > > nit: why not just take URL here (callers can still move into the parameter)? That way we know this function takes ownership of URL. I don't think this sourceURL parameter is actually moved anywhere here, or at least there are early return paths that don't take ownership over it.
Because we would like to remove registered / kept loader from m_loaders, we need to pass Loader.
>> Source/WebCore/bindings/js/ScriptModuleLoader.cpp:404 >> + rejectToPropagateNetworkError(promise.get(), ModuleFetchFailureKind::WasCanceled, "Importing a module script is canceled."_s); > > nit: I wonder if this error should be more like "Loading a module script was cancelled" or "Load for importing a module script was canceled"
I'll open a bug for that since this error message is used in usual non-worker module loaders.
>> Source/WebCore/workers/WorkerGlobalScope.cpp:282 >> + return Exception { TypeError, "importScripts cannot be used if woerker type is \"module\""_s }; > > "woerker" => "worker"
Fixed.
>> Source/WebCore/workers/WorkerOrWorkletScriptController.cpp:364 >> + // Overwrite the detailed error with a generic error. > > nit: not sure this comment is adding anything
Removed
>> Source/WebCore/workers/WorkerOrWorkletScriptController.cpp:425 >> + // Overwrite the detailed error with a generic error. > > ditto > > I wonder if we can share this code w/ above that seems to do a similar thing.
Currently, not sharable since a lot of things are different (the above is loading synchronously, and this is loading asynchronously). This one is for Worklets and the above one is for Workers. And they have different semantics to load modules.
Yusuke Suzuki
Comment 32
2021-02-20 12:30:26 PST
Committed
r273203
(
234389@main
): <
https://commits.webkit.org/234389@main
>
Aakash Jain
Comment 33
2021-02-20 14:45:59 PST
(In reply to Yusuke Suzuki from
comment #32
)
> Committed
r273203
(
234389@main
): <
https://commits.webkit.org/234389@main
>
This seems to have broken GTK build.
234389@main
failed in
https://build.webkit.org/#/builders/43/builds/332
234388@main
passed in
https://build.webkit.org/#/builders/43/builds/331
From
https://build.webkit.org/#/builders/43/builds/332/steps/8/logs/errors
: WorkerModuleScriptLoader.h:46:94: error: ‘WorkerScriptFetcher’ has not been declared
Mark Lam
Comment 34
2021-02-20 15:41:06 PST
(In reply to Aakash Jain from
comment #33
)
> (In reply to Yusuke Suzuki from
comment #32
) > > Committed
r273203
(
234389@main
): <
https://commits.webkit.org/234389@main
> > This seems to have broken GTK build. >
234389@main
failed in
https://build.webkit.org/#/builders/43/builds/332
>
234388@main
passed in
https://build.webkit.org/#/builders/43/builds/331
> > > From
https://build.webkit.org/#/builders/43/builds/332/steps/8/logs/errors
: > > WorkerModuleScriptLoader.h:46:94: error: ‘WorkerScriptFetcher’ has not been > declared
Already fixed in
http://trac.webkit.org/r273205
, but perhaps a better fix can be implemented e.g. forward declaring WorkerScriptFetcher instead of #include'ing WorkerScriptFetcher.h.
Tobias Uhlig
Comment 35
2021-04-24 02:09:25 PDT
Created
attachment 426977
[details]
Advanced use case Hi guys, I just installed the tech preview to test it with a pretty complex use case.
https://neomjs.github.io/pages/node_modules/neo.mjs/apps/covid/index.html
Looks really good. Great job! I can finally remove the warning message and add Safari to the list of supported browsers once this hits the non tech preview version (well, except for the shared worker demos). Best regards, Tobias
Tobias Uhlig
Comment 36
2021-04-24 04:22:40 PDT
I wrote a quick blog post on this one:
https://tobiasuhlig.medium.com/safari-webkit-about-to-release-support-for-js-modules-inside-the-worker-scope-9dd33fc20190?source=friends_link&sk=db61a01446ca5a51ee2f932aad9ca890
Worth a look for the dom node rotation (direction) issue. Best regards, Tobias
Yusuke Suzuki
Comment 37
2021-04-24 13:39:17 PDT
(In reply to Tobias Uhlig from
comment #35
)
> Created
attachment 426977
[details]
> Advanced use case > > Hi guys, > > I just installed the tech preview to test it with a pretty complex use case. >
https://neomjs.github.io/pages/node_modules/neo.mjs/apps/covid/index.html
> > Looks really good. Great job! > > I can finally remove the warning message and add Safari to the list of > supported browsers once this hits the non tech preview version (well, except > for the shared worker demos). > > Best regards, > Tobias
Nice! If you find an issue, please report it to bugs.webkit.org, and CC me (
ysuzuki@apple.com
)! Such a report is very welcomed, and it can improve quality significantly.
Thomas Steiner
Comment 38
2021-05-31 13:35:07 PDT
Is this feature coming to mobile Safari, too? It works like a charm on desktop Safari.
Jeff Schiller
Comment 39
2021-06-03 08:30:42 PDT
I haven't had time to test this, but if Safari supports this feature now/soon, can someone update
https://github.com/mdn/browser-compat-data/blob/main/api/Worker.json#L173
?
Thomas Steiner
Comment 40
2021-06-03 08:52:09 PDT
> I haven't had time to test this, but if Safari supports this feature now/soon, can someone update
https://github.com/mdn/browser-compat-data/blob/main/api/Worker.json#L173
?
Just opened
https://github.com/mdn/browser-compat-data/pull/10749
. No support on mobile yet.
Lucas Garron
Comment 41
2021-06-23 23:48:48 PDT
May I ask if this is expected to be available to non-Safari browsers in iOS 15? Being able to rely on ESM for workers in a web app would significantly help with compatibility and performance (compared to current workarounds), and it would be really disappointing if that option involved leaving other iOS browsers behind.
Thomas Steiner
Comment 42
2021-06-23 23:51:07 PDT
> May I ask if this is expected to be available to non-Safari browsers in iOS 15?
At least it is currently supported in the latest iOS 15.0 (19A5261w). I just checked in Chrome for iOS.
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug