Repository navigation
Angular Service Worker breaks HTML forms POST submit #36368
Description
🐞 bug report
Affected Package
The issue is caused by package @angular/service-worker
Is this a regression?
No
Description
We have an hybrid stack with old school PHP pages mixed with an Angular SPA, and the Angular app has a wildcard route redirecting all non-SPA navigations to the legacy PHP stack.
Combined with using the ngsw-bypass query param, the Angular Service Worker works great in most situations. However, it does break HTML forms POST submit, as the Service Worker would then return the app shell (even though req.method was post), causing the SPA to send a new GET request with the ngsw-bypass query param, and breaking the form submission.
To be clear, the entire reason why this is an issue for us is that we have way too many unknown PHP POST endpoints to be able to exclude them all from the navigationUrls regexes (and they often live next to actual SPA routes); the issue does not exist if the POST endpoint is excluded from navigationUrls.
Patching ngsw-worker.js at build time solves our issue, however, considering that POST requests aren't supported by the Cache API / can't really be cached by Service Workers, that upload progress events are broken as well, and that (in our case) HTML form POST submits are broken when form action targets match navigationUrls, I wonder it there's a point for the Angular Service Worker to handle POST requests in the first place.
i.e., is there any downside for the Angular Service Worker to not handle any POST requests at all like we've done?
if (req.method === 'POST' || req.headers.has('ngsw-bypass') || /[?&]ngsw-bypass(?:[=&]|$)/i.test(requestUrlObj.search)) {
return;
}
Looking through the various GitHub issues for related bugs, I feel that this would make real-life Service Worker deployments much easier.
Alternatively, I would argue that the Angular Service Worker should not return the app shell for req.mode === 'navigation' requests if req.method is not GET, in order to stay on the safe side.
🔬 Minimal Reproduction
I've created a small repro at https://github.com/laurentgoudet/angular-service-worker-submit-post-bug: just git checkout, yarn, node server.js, and open http://localhost:8080: the form submit would be broken once the Service Worker has bootstrapped
🌍 Your Environment
Angular Version:
laurent@Laurents-MacBook-Pro webapp [laurent-sw-fix] $ ./node_modules/.bin/ng version
_ _ ____ _ ___
/ \ _ __ __ _ _ _| | __ _ _ __ / ___| | |_ _|
/ △ \ | '_ \ / _` | | | | |/ _` | '__| | | | | | |
/ ___ \| | | | (_| | |_| | | (_| | | | |___| |___ | |
/_/ \_\_| |_|\__, |\__,_|_|\__,_|_| \____|_____|___|
|___/
Angular CLI: 9.0.5
Node: 12.4.0
OS: darwin x64
Angular: 9.0.5
... animations, cli, common, compiler, compiler-cli, core, forms
... language-service, localize, platform-browser
... platform-browser-dynamic, platform-server, router
... service-worker
Ivy Workspace: Yes
Package Version
-----------------------------------------------------------
@angular-devkit/architect 0.900.3
@angular-devkit/build-angular 0.900.3
@angular-devkit/build-optimizer 0.900.3
@angular-devkit/build-webpack 0.900.3
@angular-devkit/core 9.0.3
@angular-devkit/schematics 9.0.5
@angular/cdk 9.1.1
@angular/material 9.1.1
@ngtools/webpack 9.0.3
@nguniversal/builders 9.0.1
@nguniversal/common 9.0.2
@nguniversal/express-engine 9.0.2
@schematics/angular 9.0.5
@schematics/update 0.900.5
rxjs 6.5.4
typescript 3.7.5
webpack 4.41.2
Activity
laurentgoudet commentedon Apr 1, 2020
Thinking about it, the same applies to
Rangerequests or progress upload events: if we know that the Angular Service Worker (or Service Workers in general) does not handle to particular request type, IMO the Angular Service Worker should not be trying to handle it in the first place: the support for thengsw-bypassheader & query param added in #21191 only works in fairly simple applications / demo projects, when one precisely knows which type of request is used where, but is currently impractical to use in real-life large scale projects (as in you can't practically go through all your legacy code & add that header).My new
ngsw-worker.jspatch has become as below, but I think solving these issues for the community would really drive Service Workers adoption, which are a groundbreaking technology but, due to the fact that they impact all the network requests made from the SW scope (/most of the time) - not just the new shinny SPA parts - are really hard to deploy in production at the moment.laurentgoudet commentedon Apr 1, 2020
I guess the
Rangerequests part is debatable though, as one could want their Service Worker to cache videos for offline use; in practical purpose though, it's still not a great developer experience that these are broken by default IMO. Corresponding GH issue is in #25865.gkalpak commentedon Apr 1, 2020
Thx for the detailed write-up and the reproduction, @laurentgoudet 👍
Yeah,
Rangerequests are different (and could be handled by the SW). Let's keep tracking that in #25865.Regarding
POSTrequests, there is value in them going through the SW (even if just to pass through to the network in the end). For exampleDataGroupuses them to invalidate caches.I do agree, though, that it doesn't make much sense for the SW to return
index.html(aka the app shell) for non-GET requests. I.e. we could add an extra check forreq.method === 'GET'here.Would you, by any chance, be interested in submitting a pull request, @laurentgoudet?
3 remaining items
fix(service-worker): Service Worker breaks HTML forms POST submit
fix(service-worker): only consider GET requests as navigation requests
fix(service-worker): only consider GET requests as navigation requests (
angular-automatic-lock-bot commentedon Oct 7, 2022
This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.
Read more about our automatic conversation locking policy.
This action has been performed automatically by a bot.