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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add 'VITE_' prefix to SvelteKit framework #6821

Merged
merged 3 commits into from
Dec 14, 2021
Merged

Conversation

domeccleston
Copy link
Contributor

馃搵 Checklist

Since SvelteKit uses Vite under the hood, it needs the _VITE environment variable prefix. This adds it.

Example which reproduces the Svelte Kit example with undefined environment variables:

Tests

Currently I get an error when trying to run the tests: Module ts-jest in the transform option was not found.

  • The code changed/added as part of this PR has been covered with tests
  • All tests pass locally with yarn test-unit

Code Review

  • This PR has a concise title and thorough description useful to a reviewer
  • Issue from task tracker has a link to this PR

@domeccleston domeccleston marked this pull request as ready for review October 11, 2021 10:40
@codecov
Copy link

codecov bot commented Oct 11, 2021

Codecov Report

Merging #6821 (23766ec) into main (0505f4e) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##             main    #6821   +/-   ##
=======================================
  Coverage   49.17%   49.17%           
=======================================
  Files         135      135           
  Lines        5426     5426           
  Branches     1358     1358           
=======================================
  Hits         2668     2668           
  Misses       2745     2745           
  Partials       13       13           

Continue to review full report at Codecov.

Legend - Click here to learn more
螖 = absolute <relative> (impact), 酶 = not affected, ? = missing data
Powered by Codecov. Last update 0505f4e...23766ec. Read the comment docs.

@leerob
Copy link
Member

leerob commented Oct 11, 2021

You might want to check with @styfle, I believe changing the env var preset is considered a breaking change here. Did SvelteKit change this under the hood?

@raymclee
Copy link

raymclee commented Nov 6, 2021

Is it the reason I cannot get SVELTEKIT_VERCEL_URL on vercel?

Copy link
Member

@styfle styfle left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@styfle styfle added the semver: patch PR contains bug fixes label Dec 14, 2021
@TooTallNate TooTallNate merged commit f42391a into main Dec 14, 2021
@TooTallNate TooTallNate deleted the sveltekit-env-prefix branch December 14, 2021 03:12
@TooTallNate
Copy link
Member

Thanks @domeccleston!

@devunt
Copy link

devunt commented Apr 9, 2023

This should be updated to PUBLIC_ prefix, according to the newly released SvelteKit v1.0 docs [1]

[1] https://kit.svelte.dev/docs/configuration#env

@Rich-Harris
Copy link
Contributor

We actually don't want a prefix here, because it would cause the values to be available via $env/static/public and $env/dynamic/public, when they only need to be available on the server (via $env/static/private and $env/dynamic/private).

In particular, adding them to public env will mean they're included in every rendered HTML page (because that's how $env/dynamic/public is populated), which is undesirable.

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

Successfully merging this pull request may close these issues.

None yet

8 participants