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

astro:env public variables should not be checked during sync #11281

Closed
Trombach opened this issue Jun 18, 2024 · 6 comments · Fixed by #11326
Closed

astro:env public variables should not be checked during sync #11281

Trombach opened this issue Jun 18, 2024 · 6 comments · Fixed by #11326
Assignees
Labels
- P3: minor bug An edge case that only affects very specific usage (priority) feat: env related to the way astro handles `.env` or `import.meta.env` or `process.env` (scope)

Comments

@Trombach
Copy link

Trombach commented Jun 18, 2024

Describe the Bug

I'm not sure if it's best to report here or if this is an issue with astro:env.

I've moved my environment setup to astro:env and noticed that my Github action running astro-check started failing. An example can be found here. I would expect for astro-check to only infer the type of my defined variable, but not to require the variables to actually be set.

It's easy enough to work around by just setting the environment variables to something, but it would be nice if we didn't have to for simple CI checks like this.

image

Steps to Reproduce

  1. npm init astro
  2. add non optional envField to astro.config, e.g.
schema: {
  PUBLIC_VARIABLE: envField.string({
   context: "server",
   access: "public",
  }),
},
  1. run npm run astro check
  2. Error!
    The following environment variables do not match the data type and/or properties defined in experimental.env.schema:

Variable PUBLIC_VARIABLE is not of type: string.

Minimal reproduction

https://stackblitz.com/~/github.com/Trombach/astro-check-test

@github-actions github-actions bot added the needs triage Issue needs to be triaged label Jun 18, 2024
@Princesseuh
Copy link
Member

astro check implicitly runs astro sync internally (otherwise in CI environment, you wouldn't have types for content collections and stuff), the issue seems to be in the later command

@florian-lefebvre florian-lefebvre transferred this issue from withastro/language-tools Jun 18, 2024
@florian-lefebvre florian-lefebvre changed the title 🐛 BUG: astro-check fails for public variables when using astro:env astro:env public variables should not be checked during sync Jun 18, 2024
@florian-lefebvre florian-lefebvre added needs repro Issue needs a reproduction and removed needs triage Issue needs to be triaged labels Jun 18, 2024
Copy link
Contributor

Hello @Trombach. Please provide a minimal reproduction using a GitHub repository or StackBlitz. Issues marked with needs repro will be closed if they have no activity within 3 days.

@florian-lefebvre
Copy link
Member

I think this is because we check the variables during buildStart, and the vite plugin also runs during astro sync. I think we need a way to know when it's syncing and avoid throwing errors in this case? What do you think @bluwy?

@Trombach
Copy link
Author

Trombach commented Jun 18, 2024

Thanks for looking into this @Princesseuh and @florian-lefebvre. Yes, I have noticed that astro sync shows the same behaviour and it makes sense it behaves this way. I will send a minimal repro later today

@Trombach
Copy link
Author

@florian-lefebvre I've added a stackblitz to the description

@florian-lefebvre florian-lefebvre added - P3: minor bug An edge case that only affects very specific usage (priority) feat: env related to the way astro handles `.env` or `import.meta.env` or `process.env` (scope) and removed needs repro Issue needs a reproduction labels Jun 19, 2024
@florian-lefebvre florian-lefebvre self-assigned this Jun 19, 2024
@bluwy
Copy link
Member

bluwy commented Jun 20, 2024

I think this is because we check the variables during buildStart, and the vite plugin also runs during astro sync. I think we need a way to know when it's syncing and avoid throwing errors in this case? What do you think @bluwy?

Sounds ok to me, but I'm not sure how to get that info from a plugin. The way we usually handle types file generation though is directly in the sync command:

try {
await dbPackage?.typegen?.(astroConfig);
const exitCode = await syncContentCollections(settings, { ...options, logger });
if (exitCode !== 0) return exitCode;
logger.info(null, `Types generated ${dim(getTimeStat(timerStart, performance.now()))}`);
return 0;
} catch (err) {

So perhaps we should move the env generation there? And the plugin shouldn't be generating any files.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
- P3: minor bug An edge case that only affects very specific usage (priority) feat: env related to the way astro handles `.env` or `import.meta.env` or `process.env` (scope)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants