Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SvelteKit 2: Importing $env/dynamic/public in safari causes a Cannot access uninitialized variable on hydration #11364

Closed
thesiti92 opened this issue Dec 16, 2023 · 23 comments
Labels
bug Something isn't working
Milestone

Comments

@thesiti92
Copy link

thesiti92 commented Dec 16, 2023

Describe the bug

My company tried upgrading our app to SvelteKit 2, except that using it on Safari would always cause a panic both in dev mode and a production build. SvelteKit 1 still works fine.

I was trying to investigate the bug, and was able to reproduce it in dev mode by just importing $env/dynamic/public from a layout file more than once. I assume that a production build does some tree shaking or something that prevents it from being reproduced in such a contrived way.

Reproduction

This link should fail on dev mode in Safari with Cannot access uninitialized variable. All of my coworkers who tried it in Safari also experienced the same behavior.

https://stackblitz.com/edit/sveltejs-kit-template-default-ankg48?file=src%2Froutes%2F%2Bpage.svelte

Logs

[Error] ReferenceError: Cannot access uninitialized variable.
validate — exports.js:12
(anonymous function) — client.js:519

	handleError (app.js:16)
	handle_error (client.js:1485)
	(anonymous function) (client.js:2088)

System Info

Safari 16.5, Ventura 13.4

Severity

blocking an upgrade

Additional Information

I recognize this bug is similar to #2889 but we've been able to reproduce it consistently, and changing any build parameters doesnt fix it.

Any help in solving this would be appreciated, as we really want to use Shallow Routing!

@devunt
Copy link
Contributor

devunt commented Dec 16, 2023

We are having the same issue after upgrading to v2.0.0.

CleanShot 2023-12-17 at 03 42 46@2x

@jakobpesch
Copy link

I also get the error. However only in the production build and not in dev mode. 🤔 (Maybe it's a separate issue then, idk)

It also only happens during in client side navigation. The initial page load works without errors.

Safari 16.4, Ventura 13.3

image

@jakobpesch
Copy link

I fixed it for me by using $env/static/public instead of $env/dynamic/public as the migration docs suggest:

-- import { env } from '$env/dynamic/public';
++ import { PUBLIC_API_URL } from '$env/static/public';

https://kit.svelte.dev/docs/migrating-to-sveltekit-2#dynamic-environment-variables-cannot-be-used-during-prerendering

@thesiti92
Copy link
Author

Regardless of static or dynamic, it still shouldn't be causing a hard crash in only one browser by just importing it. In our production app, we need some server side secrets, so putting everything in static isn't feasible.

@jakobpesch
Copy link

jakobpesch commented Dec 19, 2023

Correct me if I'm wrong, but isn't the only difference between static and dynamic that static variables are known at build time (.env read with dotenv for instance) whereas dynamic variables are for example set in a docker-compose.

So regardless of your variables being public or private, static variables are still usable for both.

At the same time I also do not want to have to build my app for each environment separately...

But I totally agree that the behaviour should not result in a bug in one browser and not in another.

@CaptainCodeman
Copy link
Contributor

CaptainCodeman commented Dec 19, 2023

I fixed it for me by using $env/static/public instead of $env/dynamic/public as the migration docs suggest:

Those do very different things though, so be clear why you were / are using one over the other.

@thesiti92
Copy link
Author

We are explicitly using dynamic since we don't own our build servers and do not want to leak our secrets at build time, so we can't use these workarounds.

We just need someone to look into why importing some packages break SvelteKit2 in Safari.

@marcusirgens
Copy link

marcusirgens commented Dec 20, 2023

Here are some debugger screenshots from Safari, running SvelteKit 2 in dev mode, I'm seeing the same issue.

This is in load_node (source)
Screenshot 2023-12-20 at 15 54 38

This is in validate(source)
Screenshot 2023-12-20 at 16 01 29

@elliott-with-the-longest-name-on-github
Copy link
Contributor

We are explicitly using dynamic since we don't own our build servers and do not want to leak our secrets at build time, so we can't use these workarounds.

I'm just catching up here, but this sounds pretty fishy. Using $env/dynamic/public doesn't fix your issue -- anything that goes in that module is, er, public. It's not secret. There's no difference, leakiness-wise, between using $env/static/public on the build server and $env/dynamic/public to populate from your webserver -- in both cases, you should only serve public variables through here.

@aldelucca1
Copy link

aldelucca1 commented Dec 22, 2023

I'm just catching up here, but this sounds pretty fishy. Using $env/dynamic/public doesn't fix your issue -- anything that goes in that module is, er, public. It's not secret. There's no difference, leakiness-wise, between using $env/static/public on the build server and $env/dynamic/public to populate from your webserver -- in both cases, you should only serve public variables through here.

We are using both $env/dynamic/public and $env/dynamic/private. Please see the minimal reproduction above, as it can be reproduced in its simplest form by indirectly including the $env/dynamic/public module more than once.

Regardless of the possibility of working around the issue, it seems like there may be a bug in how the generated javascript in the minimal case above is being output with regard to Safari

@Coronon
Copy link

Coronon commented Dec 27, 2023

I also experienced this bug which causes very irrational behavior on Safari. Sometimes even importing a random, empty component will break the site while importing another empty component on another page will fix it again. I just spent a lot of time debugging this to reach the same conclusion... ^^

@thesiti92
Copy link
Author

@Coronon Can you link another stackblitz example that I can add to the issue and change the title? I think that may help a maintainer actually take a look at this! Thanks!

eXodes added a commit to eXodes/releve that referenced this issue Dec 29, 2023
eXodes added a commit to eXodes/releve that referenced this issue Dec 29, 2023
* feat: vite pwa manifest

* feat: pwa images

* fix: app metadata

* fix: mobile nav slider

* chore: prettier format

* fix: vite pwa type

* fix: vite pwa config

* chore: ignore generated sw

* chore: update env variables

* fix: update env variables to public

* build: sentry source maps config

* fix: sentry init release

* ci: update env for deployment

* ci: sentry env for upload

* build: set build target to modules

* build: revert vite build target

* fix: dynamic env import

Ref: sveltejs/kit#11364

* ci: missing build env

* chore: missing test env

* ci: sentry release ref

* fix: banner position for standalone

* fix: add new shop z-index

* fix: add new shop float button

* fix: app banner standalone style

* chore: check release value

* fix: listbox with popper

* build: update svelte-headlessui version

* chore: github reference

* ci: sentry release with github ref

* fix: form store for binding
@Coronon
Copy link

Coronon commented Jan 1, 2024

@thesiti92 Happy new year and sorry for the late reply. I attempted a short recreation but wasn't successful yet. Sadly I'm a little short on time this and next week :( If the issue is still open after that (let's hope not) I'll see what I can do :)

@bfanger
Copy link
Contributor

bfanger commented Jan 6, 2024

In 9439190 the exports/vite/index.js the line:

  return `export const env = ${global}.env;`;

was changed into:

  return `export const env = ${global}.env ?? (await import(/* @vite-ignore */ ${global}.base + '/' + '${kit.appDir}/env.js')).env;`;

this is causing the ReferenceError: Cannot access uninitialized variable. in Safari/Webkit

@denjpeters
Copy link

We have the same error (Unhandled Promise Rejection: ReferenceError: Cannot access uninitialized variable.) on upgrading from SK1 to SK2.

It is occurring in our npmjs library app where we are having the problem (and is propagating to our production apps). That library app does NOT use ANY env variables. So, I wonder if that is a red herring.

Some routes work fine, but others experience this problem EVERY time when first navigating to them, but navigating somewhere else, and then back shows the page correctly.

We have been trying to track down what offending code in our app is doing this, but still haven't been able to figure it out.

Note: I've copied the offending code below. The error occurs on the line with the 'constructors' prop. I'm guessing the issue might have something to do with the forced @ts-ignore on the line above:

props: {
// @ts-ignore Somehow it's getting SvelteComponent and SvelteComponentDev mixed up
constructors: compact(branch).map((branch_node) => branch_node.node.component),
page
}

@thesiti92
Copy link
Author

Yes the error likely has to do with some module loading code sk2 outputs, but I found a minimal reproduction with the env package. Because it obviously has to do with sveltekit internals, it would be helpful if a contributor could help debug it!

@bfanger
Copy link
Contributor

bfanger commented Jan 7, 2024

Safari has a bug in its global await implementation, as #7805 points out, the following code fails when importing the dependency more than once.

export let bar = 'foo'
await 0

This is why importing "$env/dynamic/public" is causing issues in SvelteKit v2 in Safari

@IAkumaI
Copy link
Contributor

IAkumaI commented Jan 8, 2024

Same issue. Just switched my project to sveltekit :)
The bug shows itself only in production, so I just hadn’t seen it while testing. Sadly, pity safari users.

@denjpeters
Copy link

Any update on this issue?

I can confirm that it is not just Safari for Mac that experiences this. It breaks iOS devices as well.

@Rich-Harris
Copy link
Member

fix here: #11601

i don't love it, but i like it a lot more than the alternative fix i briefly tried using sync XHR... 🤮

Rich-Harris added a commit that referenced this issue Jan 11, 2024
@Rich-Harris
Copy link
Member

fixed in 2.3.2

@denjpeters
Copy link

Thanks for the fix! Works like a charm!

@thesiti92
Copy link
Author

yes! thank you! we were able to upgrade to SvelteKit2 like a charm

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.