wouter icon indicating copy to clipboard operation
wouter copied to clipboard

don't use global location, because it doesn't work on cloudflare workers

Open limikael opened this issue 1 year ago • 1 comments

In a couple of places there are references to location, which on a browser reads the location from window.location. In an SSR environment, this reads from global.location which isn't a problem on node.js, because it will just be undefined, and isn't used when doing SSR anyway. However, on Cloudflare Workers there isn't any global object at all, so the code trying to read from location tries to read a global variable named location, which doesn't exist, so the code crashes due to an undefined variable. It is possible to enable a special Node.js compatibility mode, so the global will exist and location will be undefined rather than non-existent, but this compatibility mode gives the warning:

Enabling Node.js compatibility mode for built-ins and globals. This is experimental and has serious tradeoffs. Please see https://github.com/ionic-team/rollup-plugin-node-polyfills/ for more details.

So far I have been using the compatibility mode to do SSR rendering, but it would be nice to be able to turn it off! This PR adds checks with typeof location!="undefined" before trying to read the variables so it works without combatibility mode.

Also, I had problems when I was doing "multipass SSR". By "multipass SSR" I mean that I was doing external fetching with promises in the SSR, then I waited for the promises to resolve, and did SSR again. When doing this, the ssrPath parameter was not respected for the second pass. So I also changed the semantics slightly for the ssrPath parameter. Now, if the ssrPath parameter is set, it will always be used.

limikael avatar Sep 24 '23 10:09 limikael

@limikael Thank you for reporting this. I wonder why this problem happens, because in Node.js location is also undefined (even though it can be accessed globally). For example, the function const currentSearch = () => location.search; wouldn't work in node either, it would throw Cannot read properties of undefined .

Normally, when ssrPath is present the code that refers to location isn't invoked at all. This is done by React, the hook useSyncExternalStore accepts second parameter (see getServerSnapshot) that is a callback that will be called when the app is rendered on the server.

My guess would be that it happens during the compile time (can you confirm that?), but I believe I need to do proper investigation.

In the meantime, I'll take a look at your PR, thank you!

molefrog avatar Sep 27 '23 10:09 molefrog

Closing this as it's been fixed in v3. Thank you!

molefrog avatar Feb 26 '24 20:02 molefrog