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

Dynamic Public Environment Variables are undefined in +server.ts endpoints #5941

Closed
ssbm-oro opened this issue Aug 16, 2022 · 3 comments · Fixed by #5955
Closed

Dynamic Public Environment Variables are undefined in +server.ts endpoints #5941

ssbm-oro opened this issue Aug 16, 2022 · 3 comments · Fixed by #5955

Comments

@ssbm-oro
Copy link

Describe the bug

Hello, I want to start off by thanking everyone involved for their hard work and clever ideas that have brought Svelte and Sveltekit to the points that the projects are at now 🥰

I updated to the latest version of Sveltekit today, and after going through all the migrations and fixing everything, I have one remaining problem in my project, that dynamic public environment variables are not available in +server.ts endpoints.

I made a simple example repository showing the issue. After you install and build it, you can see that the public dynamic is rendered correctly when you navigate to /

If you navigate to /static, the endpoint will return a public and a private static environment variable and display 'working foo'.

If you navigate to /dynamic, the endpoint will return a public and private dynamic environment variable, and should display 'working bar'. However, the public dynamic variable is undefined, so it displays 'undefined bar'.

Reproduction

https://github.com/ssbm-oro/sveltekit-dynamic-public-bug

Logs

No response

System Info

System:
    OS: macOS 12.5
    CPU: (8) arm64 Apple M2
    Memory: 78.63 MB / 24.00 GB
    Shell: 5.8.1 - /bin/zsh
  Binaries:
    Node: 18.6.0 - /opt/homebrew/bin/node
    npm: 8.13.2 - /opt/homebrew/bin/npm
  Browsers:
    Firefox: 103.0.1
    Safari: 15.6
  npmPackages:
    @sveltejs/adapter-auto: next => 1.0.0-next.64 
    @sveltejs/kit: next => 1.0.0-next.411 
    svelte: ^3.44.0 => 3.49.0 
    vite: ^3.0.0 => 3.0.8

Severity

annoyance

Additional Information

No response

@Rich-Harris
Copy link
Member

Braindumping my investigation so far: this is happening because vite.ssrLoadModule is loading two instances of both $env/dynamic/private and $env/dynamic/public. One is loaded by the plugin itself, in order to call the internal set_public_env and set_private_env functions before the app starts, and a second instance is loaded by the module that imports it. That second instance has env = {}, because its exported set_[mode]_env function is never called.

In theory they should be resolved to the same place, and Vite should just re-use the existing module. For some reason (possibly a bug in Vite? not 100% sure) that's not happening.

Due to a quirk in the module resolution algorithm, this doesn't happen inside the sveltejs/kit repo, only in apps that have the @sveltejs/kit package installed inside node_modules.

@bluwy
Copy link
Member

bluwy commented Aug 17, 2022

In theory they should be resolved to the same place, and Vite should just re-use the existing module. For some reason (possibly a bug in Vite? not 100% sure) that's not happening.

I don't quite understand this part. Are we importing the modules with two different URLs that should ideally be deduped to one single instance? I can't find how this is happening in the codebase, maybe I'm missing something.

@aloker
Copy link
Contributor

aloker commented Aug 19, 2022

#5955 does not fix this issue on Windows. Here are my findings:

next.405

  • $env/dynamic/public and $env/dynamic/private work as expected

next.408

  • both env-objects in $env/dynamic/public and $env/dynamic/private are empty on the server side, $env/dynamic/public work on the client side

next.418

  • fixes the issues for SSR on Linux, but not on Windows

next.427

  • Still broken for Windows

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants