Change how module scripts are fetched #3656

annevk · 2018-04-30T09:57:49Z

Module scripts are always fetched with credentials mode "same-origin" by default and can no longer
use "omit". This makes them match other high-level platform features.
The top-level script for module workers is always fetched with request mode "same-origin" and
credentials mode "same-origin". Cross-origin workers did not quite work due to service workers.
Any module worker import scripts are fetched with request credentials mode "same-origin" and this
cannot be configured for now.

Fixes #2557 and fixes #3109.

/urls-and-fetching.html ( diff )
/webappapis.html ( diff )
/workers.html ( diff )

annevk · 2018-04-30T09:58:18Z

@domenic can you help me locate the tests for these features? (And maybe help tweak them.)

domenic · 2018-04-30T17:03:11Z

Thanks very much for taking this on.

Any particular reason this removes the credentials mode feature from the Worker constructor instead of just changing the default? That removes two abilities in particular:

The ability to set the credentials mode to omit. (Not present in type=module after this change.)
The ability to set the credentials mode to include for imports. (Which you can do with type=module via crossorigin=use-credentials)

All else being equal I'd rather keep that option...

/cc @nhiroki @mattto for tracking as they are implementing currently.

Tests might be best uncovered by someone actually implementing this (perhaps @domfarolino or @hiroshige-g are interested) and seeing what breaks. The tests are all in https://github.com/w3c/web-platform-tests/tree/master/html/semantics/scripting-1/the-script-element/module . The crossorigin-* ones are the only ones that seem relevant from first scan...

annevk · 2018-04-30T17:17:06Z

I don't think we should offer the ability to use credentials mode "omit" outside fetch() until we tackle it more generally (see #2614). We could keep the ability to use "include" with a more restricted enum I suppose. It wasn't entirely clear to me if v1 needed that as we don't really have use cases other than the worry about connection sharing in non-Firefox browsers.

domenic · 2018-04-30T17:25:10Z

I guess we can remove it, I just worry that people are going to be complaining and then we'll just add everything back.

I'm not sure this would be blocked on #2614 since solving omit for the markup use case is pretty different from solving it for the API use case. We already have API precedent with fetch().

annevk · 2018-04-30T17:28:04Z

The API argument is somewhat interesting, but we don't have any other API offer this functionality today, which is why I'm hesitant (but also fine with adding it if folks will implement it). E.g., EventSource does not.

Does service workers require changes here?

cc @jungkees @jakearchibald

nhiroki · 2018-05-01T12:51:00Z

Worklets have credentials option that defaults to omit and WPTs for the option.

Modules workers don't have WPTs for the credentials option yet.

Filed an issue in the chromium bug tracker: https://crbug.com/838540

annevk · 2018-05-02T07:37:31Z

cc @wanderview @jonco3

This CL adds web-platform-tests for the "credentials" option of WorkerOptions: https://html.spec.whatwg.org/multipage/workers.html#workeroptions The current spec defines that the default value of this option is "omit", but there is an ongoing spec discussion to change it to "same-origin": whatwg/html#3656 This CL adds the tests based on the current spec, and a subsequent CL will update them based on the decision. Bug: 843875 Change-Id: I50eb0c7971587b9d84865498d67abef8ed2d8fc6

This CL adds web-platform-tests for the "credentials" option of WorkerOptions: https://html.spec.whatwg.org/multipage/workers.html#workeroptions The current spec defines that the default value of this option is "omit", but there is an ongoing spec discussion to change it to "same-origin": whatwg/html#3656 This CL adds the tests based on the current spec, and a subsequent CL will update them based on the decision. Bug: 843875 Change-Id: I50eb0c7971587b9d84865498d67abef8ed2d8fc6 Reviewed-on: https://chromium-review.googlesource.com/1063524 Commit-Queue: Hiroki Nakagawa <nhiroki@chromium.org> Reviewed-by: Matt Falkenhagen <falken@chromium.org> Reviewed-by: Kouhei Ueno <kouhei@chromium.org> Cr-Commit-Position: refs/heads/master@{#559888}

nhiroki · 2018-05-21T06:51:28Z

Reg: #3656 (comment)

Worklets have credentials option that defaults to omit and WPTs for the option.

I changed the Worklet spec and WPTs (w3c/css-houdini-drafts#756)

Modules workers don't have WPTs for the credentials option yet.

I added WPTs for the credentials param of WorkerOptions for dedicated workers based on the current spec (i.e., omit is the default value) (web-platform-tests/wpt@d9a5af2). I'll update the tests based on the conclusion of this PR.

annevk · 2018-05-22T07:48:25Z

Okay, I guess I should bring the credentials member back in this PR and then we can proceed? @domenic?

…ted workers, a=testonly Automatic update from web-platform-testsWorker: Add credentials tests for dedicated workers This CL adds web-platform-tests for the "credentials" option of WorkerOptions: https://html.spec.whatwg.org/multipage/workers.html#workeroptions The current spec defines that the default value of this option is "omit", but there is an ongoing spec discussion to change it to "same-origin": whatwg/html#3656 This CL adds the tests based on the current spec, and a subsequent CL will update them based on the decision. Bug: 843875 Change-Id: I50eb0c7971587b9d84865498d67abef8ed2d8fc6 Reviewed-on: https://chromium-review.googlesource.com/1063524 Commit-Queue: Hiroki Nakagawa <nhiroki@chromium.org> Reviewed-by: Matt Falkenhagen <falken@chromium.org> Reviewed-by: Kouhei Ueno <kouhei@chromium.org> Cr-Commit-Position: refs/heads/master@{#559888} -- wpt-commits: d9a5af281709894851abcaff5c3af967dc0b474e wpt-pr: 11040

domenic · 2018-05-22T13:46:41Z

That would work great for me; matching worklets makes a lot of sense.

annevk · 2018-05-23T08:25:51Z

Updated the commit as well as its description.

domenic

I am still not really comfortable with removing cross-origin workers, but if @nhiroki is OK with closing #3112 in the negative, then I guess I will concede, in which case this LGTM. Looking forward to hearing his take.

annevk · 2018-05-23T13:42:36Z

@domenic given that you haven't replied to the concerns raised in that issue as well as the concerns raised in #3109 for many months that's rather frustrating.

domenic · 2018-05-23T14:13:02Z

I've replied; I think we just have different views on the relative importance of different issues, and the path forward.

nhiroki · 2018-05-28T12:54:47Z

Hmm..., this is a tough topic. I'm still weighing up the options. Cross-origin worker looks appealing. It allows to directly load library workers hosted on CDNs without a same-origin skeleton script. On the other hand, it introduces complication around Clients API, Service Worker interception, security features, etc. Maybe I'm slightly leaning to no support of the cross-origin worker for now because of the complication.

Regarding the cumbersome skeleton script, JS Blocks could mitigate it like this, I'm not sure how to determine a controller service worker in this case though.

const block = { import "https://other.example.com/worker-lib.js"; };
const worker = new worker(block, { type: 'module' });

Or, LAPIs could provide worker libraries commonly used??

annevk · 2018-10-09T09:47:52Z

Commit message looks great, thanks for that!

This CL changes the default classic script fetch options credentials mode from "omit" to "same-origin", as per the recent spec change [1], and adds descendant worker credentials tests as a follow-up to said spec change and [2]. [1]: whatwg/html#3656 [2]: #13426 R=domenic@chromium.org, kouhei@chromium.org, nhiroki@chromium.org Bug: 849101 Change-Id: I958f552f0ee91beb8aab98269f79a1eb219fb40a

This CL changes the default classic script fetch options credentials mode from "omit" to "same-origin", as per the recent spec change [1], and adds descendant worker credentials tests as a follow-up to said spec change and [2]. [1]: whatwg/html#3656 [2]: web-platform-tests/wpt#13426 R=domenic@chromium.org, kouhei@chromium.org, nhiroki@chromium.org Bug: 849101 Change-Id: I958f552f0ee91beb8aab98269f79a1eb219fb40a Reviewed-on: https://chromium-review.googlesource.com/c/1301964 Commit-Queue: Dominic Farolino <domfarolino@gmail.com> Reviewed-by: Kouhei Ueno <kouhei@chromium.org> Reviewed-by: Hiroki Nakagawa <nhiroki@chromium.org> Cr-Commit-Position: refs/heads/master@{#604862}

This CL changes the default classic script fetch options credentials mode from "omit" to "same-origin", as per the recent spec change [1], and adds descendant worker credentials tests as a follow-up to said spec change and [2]. [1]: whatwg/html#3656 [2]: #13426 R=domenic@chromium.org, kouhei@chromium.org, nhiroki@chromium.org Bug: 849101 Change-Id: I958f552f0ee91beb8aab98269f79a1eb219fb40a Reviewed-on: https://chromium-review.googlesource.com/c/1301964 Commit-Queue: Dominic Farolino <domfarolino@gmail.com> Reviewed-by: Kouhei Ueno <kouhei@chromium.org> Reviewed-by: Hiroki Nakagawa <nhiroki@chromium.org> Cr-Commit-Position: refs/heads/master@{#604862}

…ons credentials mode, a=testonly Automatic update from web-platform-testsChange default classic script fetch options credentials mode This CL changes the default classic script fetch options credentials mode from "omit" to "same-origin", as per the recent spec change [1], and adds descendant worker credentials tests as a follow-up to said spec change and [2]. [1]: whatwg/html#3656 [2]: web-platform-tests/wpt#13426 R=domenic@chromium.org, kouhei@chromium.org, nhiroki@chromium.org Bug: 849101 Change-Id: I958f552f0ee91beb8aab98269f79a1eb219fb40a Reviewed-on: https://chromium-review.googlesource.com/c/1301964 Commit-Queue: Dominic Farolino <domfarolino@gmail.com> Reviewed-by: Kouhei Ueno <kouhei@chromium.org> Reviewed-by: Hiroki Nakagawa <nhiroki@chromium.org> Cr-Commit-Position: refs/heads/master@{#604862} -- wpt-commits: 085c8a8f6eab33c6586d4d1f9897b700f41c8f54 wpt-pr: 13739

…ted workers, a=testonly Automatic update from web-platform-testsWorker: Add credentials tests for dedicated workers This CL adds web-platform-tests for the "credentials" option of WorkerOptions: https://html.spec.whatwg.org/multipage/workers.html#workeroptions The current spec defines that the default value of this option is "omit", but there is an ongoing spec discussion to change it to "same-origin": whatwg/html#3656 This CL adds the tests based on the current spec, and a subsequent CL will update them based on the decision. Bug: 843875 Change-Id: I50eb0c7971587b9d84865498d67abef8ed2d8fc6 Reviewed-on: https://chromium-review.googlesource.com/1063524 Commit-Queue: Hiroki Nakagawa <nhirokichromium.org> Reviewed-by: Matt Falkenhagen <falkenchromium.org> Reviewed-by: Kouhei Ueno <kouheichromium.org> Cr-Commit-Position: refs/heads/master{#559888} -- wpt-commits: d9a5af281709894851abcaff5c3af967dc0b474e wpt-pr: 11040 UltraBlame original commit: 8aeedb450c81a5bd7cb78599882382d2e66cc3d0

…ption from 'omit' to 'same-origin', a=testonly Automatic update from web-platform-testsWorker: Change the default credentials option from 'omit' to 'same-origin' This is a follow-up for the spec change: whatwg/html#3656 Note that ES Modules for dedicated workers is still behind the flag and this change doesn't affect applications in the real world. Bug: 848247 Change-Id: I25083f3f11f9d13663e16e2f4c137095e9b12b01 Reviewed-on: https://chromium-review.googlesource.com/1080668 Reviewed-by: Matt Falkenhagen <falkenchromium.org> Commit-Queue: Hiroki Nakagawa <nhirokichromium.org> Cr-Commit-Position: refs/heads/master{#563465} -- wpt-commits: 72c7cfed684554c0d65b7a33d413df9ae1c5999f wpt-pr: 11274 UltraBlame original commit: 8501df66d1b82cd635660d7868271fc02bd2f0fb

…ons credentials mode, a=testonly Automatic update from web-platform-testsChange default classic script fetch options credentials mode This CL changes the default classic script fetch options credentials mode from "omit" to "same-origin", as per the recent spec change [1], and adds descendant worker credentials tests as a follow-up to said spec change and [2]. [1]: whatwg/html#3656 [2]: web-platform-tests/wpt#13426 R=domenicchromium.org, kouheichromium.org, nhirokichromium.org Bug: 849101 Change-Id: I958f552f0ee91beb8aab98269f79a1eb219fb40a Reviewed-on: https://chromium-review.googlesource.com/c/1301964 Commit-Queue: Dominic Farolino <domfarolinogmail.com> Reviewed-by: Kouhei Ueno <kouheichromium.org> Reviewed-by: Hiroki Nakagawa <nhirokichromium.org> Cr-Commit-Position: refs/heads/master{#604862} -- wpt-commits: 085c8a8f6eab33c6586d4d1f9897b700f41c8f54 wpt-pr: 13739 UltraBlame original commit: 16087e3eb569c701d67e7458fbfd7ada4c318d6b

Reflecting whatwg/html#3656 (i.e. the credentials mode of https://html.spec.whatwg.org/C/#default-classic-script-fetch-options was changed to same-origin). This CL affects credentials mode used in dynamic import() executed from: - setTimeout("import()") on Window Note: this CL changes the credentials mode used in this case to same-origin, but actually it should be plumbed from initiating scripts (crbug.com/1133238). - javascript: URL - Scripts in isolated worlds Bug: 1114988, 1133238 Change-Id: I87c7083bce99276e6de05745f79bf4e2a0ba6de6 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2409895 Commit-Queue: Hiroshige Hayashizaki <hiroshige@chromium.org> Reviewed-by: Kouhei Ueno <kouhei@chromium.org> Reviewed-by: Dominic Farolino <dom@chromium.org> Cr-Commit-Position: refs/heads/master@{#818505}

Reflecting whatwg/html#3656 (i.e. the credentials mode of https://html.spec.whatwg.org/C/#default-classic-script-fetch-options was changed to same-origin). This CL affects credentials mode used in dynamic import() executed from: - setTimeout("import()") on Window Note: this CL changes the credentials mode used in this case to same-origin, but actually it should be plumbed from initiating scripts (crbug.com/1133238). - javascript: URL - Scripts in isolated worlds Bug: 1114988, 1133238 Change-Id: I87c7083bce99276e6de05745f79bf4e2a0ba6de6 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2409895 Commit-Queue: Hiroshige Hayashizaki <hiroshige@chromium.org> Reviewed-by: Kouhei Ueno <kouhei@chromium.org> Reviewed-by: Dominic Farolino <dom@chromium.org> Cr-Commit-Position: refs/heads/master@{#818505} GitOrigin-RevId: 314164e7cd5c59ca0aea95739f29d5634fe98817

annevk added topic: workers topic: fetch needs tests Moving the issue forward requires someone to write tests topic: script labels Apr 30, 2018

annevk mentioned this pull request May 2, 2018

Change credentials mode default w3c/css-houdini-drafts#756

Closed

chromium-wpt-export-bot mentioned this pull request May 17, 2018

Worker: Add credentials tests for dedicated workers web-platform-tests/wpt#11040

Merged

annevk force-pushed the annevk/module-scripts-and-credentials branch from a9a2724 to b835449 Compare May 23, 2018 08:25

domenic reviewed May 23, 2018

View reviewed changes

since this only affects import(), make it same-origin

b42f2fa

domenic merged commit 5fabb58 into master Oct 9, 2018

domenic deleted the annevk/module-scripts-and-credentials branch October 9, 2018 20:38

chromium-wpt-export-bot mentioned this pull request Oct 26, 2018

Change default classic script fetch options credentials mode web-platform-tests/wpt#13739

Merged

trotyl mentioned this pull request Sep 13, 2019

Edge requires crossorigin="anonymous" to get script files from CDN angular/angular#32660

Closed

ajbouh mentioned this pull request Aug 12, 2020

Replace modulepreload with preload for Firefox and Safari sveltejs/sapper#1387

Open

nicolo-ribaudo mentioned this pull request Jul 4, 2023

Use appropriate fetch destination for JSON&CSS modules #9486

Merged

7 tasks

sapphi-red mentioned this pull request Nov 20, 2023

DOMException when using web worker during dev with backend integration vitejs/vite#13680

Open

7 tasks

github-actions bot mentioned this pull request Apr 16, 2024

vite/5.2.9 package update wolfi-dev/os#16881

Merged

sapphi-red mentioned this pull request Aug 21, 2024

Change the default value of credential option of Worker constructor mdn/content#35539

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Change how module scripts are fetched #3656

Change how module scripts are fetched #3656

annevk commented Apr 30, 2018 •

edited by pr-preview bot

Loading

annevk commented Apr 30, 2018

domenic commented Apr 30, 2018 •

edited

Loading

annevk commented Apr 30, 2018

domenic commented Apr 30, 2018

annevk commented Apr 30, 2018 •

edited

Loading

nhiroki commented May 1, 2018 •

edited

Loading

annevk commented May 2, 2018

nhiroki commented May 21, 2018

annevk commented May 22, 2018

domenic commented May 22, 2018

annevk commented May 23, 2018

domenic left a comment

annevk commented May 23, 2018

domenic commented May 23, 2018

nhiroki commented May 28, 2018 •

edited

Loading

annevk commented Oct 9, 2018

Change how module scripts are fetched #3656

Change how module scripts are fetched #3656

Conversation

annevk commented Apr 30, 2018 • edited by pr-preview bot Loading

annevk commented Apr 30, 2018

domenic commented Apr 30, 2018 • edited Loading

annevk commented Apr 30, 2018

domenic commented Apr 30, 2018

annevk commented Apr 30, 2018 • edited Loading

nhiroki commented May 1, 2018 • edited Loading

annevk commented May 2, 2018

nhiroki commented May 21, 2018

annevk commented May 22, 2018

domenic commented May 22, 2018

annevk commented May 23, 2018

domenic left a comment

Choose a reason for hiding this comment

annevk commented May 23, 2018

domenic commented May 23, 2018

nhiroki commented May 28, 2018 • edited Loading

annevk commented Oct 9, 2018

annevk commented Apr 30, 2018 •

edited by pr-preview bot

Loading

domenic commented Apr 30, 2018 •

edited

Loading

annevk commented Apr 30, 2018 •

edited

Loading

nhiroki commented May 1, 2018 •

edited

Loading

nhiroki commented May 28, 2018 •

edited

Loading