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.

Sign up for GitHub

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

Jump to bottom

feat: allow access to public env in app.html #8449

Merged
merged 12 commits into from
Jan 19, 2023
Merged

feat: allow access to public env in app.html #8449

merged 12 commits into from
Jan 19, 2023

Conversation

chrskerr
Copy link
Contributor

@chrskerr chrskerr commented Jan 11, 2023 •
edited by dummdidumm
Loading

closes #8427

This PR is to enable access to public env variables within app.html at render-time.

An example of this being useful would be the insertion of a public api key into a <script> based upon the deploy environment configuration.

Please don't delete this checklist! Before submitting the PR, please make sure you do the following:

  • It's really useful if your PR references an issue where it is discussed ahead of time. In many cases, features are absent for a reason. For large changes, please create an RFC: https://github.com/sveltejs/rfcs
  • This message body should clearly illustrate what problems it solves.
  • Ideally, include a test that fails without this PR but passes with it.

Tests

  • Run the tests with pnpm test and lint the project with pnpm lint and pnpm check

Changesets

  • If your PR makes a change that should be noted in one or more packages' changelogs, generate a changeset by running pnpm changeset and following the prompts. All changesets should be patch until SvelteKit 1.0

@changeset-bot
Copy link

changeset-bot bot commented Jan 11, 2023 •
edited
Loading

🦋 Changeset detected

Latest commit: 30b353b

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@sveltejs/kit Minor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@chrskerr chrskerr marked this pull request as ready for review January 11, 2023 01:44
dummdidumm
dummdidumm requested changes Jan 11, 2023
View reviewed changes
Copy link
Member

@dummdidumm dummdidumm left a comment

Choose a reason for hiding this comment

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

Thank you! To finish this, you need to add the same logic to build_server.js where the app_template function is built for the production build. I'd hold off from doing this though because Rich is currently refactoring that part of the app in #8429 - best wait until he's done with that.

@benmccann benmccann changed the title [feat] Allow access to public env in app.html feat: allow access to public env in app.html Jan 11, 2023
@chrskerr
Copy link
Contributor Author

Thank you! To finish this, you need to add the same logic to build_server.js where the app_template function is built for the production build. I'd hold off from doing this though because Rich is currently refactoring that part of the app in #8429 - best wait until he's done with that.

Thanks @dummdidumm @benmccann I have subscribed to that PR and will update this one when that's merged in 😃

@chrskerr chrskerr force-pushed the public-envs-in-app.html branch from 3f3ef5a to 2315698 Compare January 13, 2023 22:40
@Rich-Harris Rich-Harris merged commit 2726e7c into sveltejs:master Jan 19, 2023
@Rich-Harris
Copy link
Member

thank you!

@github-actions github-actions bot mentioned this pull request Jan 19, 2023
Merged
@cedricr cedricr mentioned this pull request Feb 10, 2023
Merged
cedricr added a commit to gip-inclusion/dora-front that referenced this pull request Feb 10, 2023
@cedricr
Corrige les erreurs de Content-Length des versions récentes de Svelte…
439133a 
…Kit (#184)

Après le passage de SvelteKit de 1.0.0 en 1.5.2, Chrome reportait des
erreurs de Content-Length systématiques, qui bloquaient le reste du
chargement.

Le changement en cause: sveltejs/kit#8371
SvelteKit, depuis la version 1.2.0, rajoute un en-tête Content-Length à
la réponse, or dans

https://github.com/betagouv/dora-front/blob/571f7b9033510dec8acd362c8825650c755fc5c7/src/hooks.server.ts#L34
on réécrivait le contenu sans mettre à jour sa taille.

Depuis 1.2.0, il est également possible d'accéder aux variables
d'environnement directement dans le app.html, ce qui était la raison de
la réécriture jusque là:
sveltejs/kit#8449
Cette PR remplace donc l'ancien mécanisme de réécriture par l'officiel.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dummdidumm dummdidumm dummdidumm requested changes

@Rich-Harris Rich-Harris Rich-Harris approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
4 participants
@chrskerr @Rich-Harris @dummdidumm @benmccann