seanmorris / php-wasm

PHP in Browser, powered by WebAssembly.
https://php-wasm.seanmorr.is/
Apache License 2.0
781 stars 41 forks source link

handleFetchEvent should not perform a fetch #64

Closed mglaman closed 1 month ago

mglaman commented 1 month ago

Per https://developer.mozilla.org/en-US/docs/Web/API/FetchEvent, event listeners for FetchEvent should return responses using respondWith or do nothing. The code uses respondWith for the PHP response, but then there is an else and a fetch performed. This is doubling the request.

Need to remove

        else
        {
            return fetch(event.request);
        }
mglaman commented 1 month ago

Looks like this problem was attempted to workaround here:

            isWhitelisted = url.pathname.substr(0, prefix.length) === prefix && url.hostname === self.location.hostname;

            isBlacklisted = url.pathname.match(/\.wasm$/i)
            || staticUrls.includes(url.pathname)
            || (this.exclude.findIndex(exclude => url.pathname.substr(0, exclude.length) === exclude) > -1)
            || false;
seanmorris commented 1 month ago

@mglaman

That's something Patricio and I have been going over. We need away to get through the service worker and put a request all the way through to the actual server in some cases.

Are you suggesting we remove the fetch call alltogether, or use respondWith instead of return?

mglaman commented 1 month ago

Remove the fetch or use respondWith. I'd opt for full removal and allow normal browser behavior