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

Vite's define dev-mode timing issues in .astro files #11874

Closed
1 task done
AriPerkkio opened this issue Aug 29, 2024 · 4 comments
Closed
1 task done

Vite's define dev-mode timing issues in .astro files #11874

AriPerkkio opened this issue Aug 29, 2024 · 4 comments
Labels
needs triage Issue needs to be triaged

Comments

@AriPerkkio
Copy link

Astro Info

Astro                    v4.15.0
Node                     v20.15.0
System                   macOS (arm64)
Package Manager          pnpm
Output                   static
Adapter                  none
Integrations             none

If this issue only occurs in one browser, which browser is a problem?

No response

Describe the Bug

In development mode, it's possible that values defined in Vite's define are not present in .astro files immediately. Note that there is a difference in Vite how define works when comparing dev and prod modes - prod uses static replacements, dev adds them to globalThis on runtime.

Maybe Astro is loading .astro files before Vite defines values from define? Is Astro now too fast?

import { defineConfig } from "astro/config";

export default defineConfig({
  vite: {
    define: {
      __MY_DEFINE__: '"Value set in config"',
    },
  },
});
---
const frontmatter = __MY_DEFINE__;
---

<h1>My Component</h1>

<script define:vars={{frontmatter}}>
  console.log({frontmatter, scriptScope: globalThis.__MY_DEFINE__})

  setTimeout(() => {
    console.log({frontmatter, scriptScope: globalThis.__MY_DEFINE__})
  }, 1000)
</script>

image

What's the expected result?

Values defined in Vite's define should always be present, even in .astro files. The define option should provide similar results in dev as it does in prod.

Link to Minimal Reproducible Example

https://stackblitz.com/~/edit/github-5kyhn9?file=src%2Fpages%2Findex.astro

Participation

  • I am willing to submit a pull request for this issue.
@matthewp
Copy link
Contributor

Hi @AriPerkkio, what is happening here is that because you are using define:vars this script is inlined. It runs where ever it is placed on the page, and will always run before Vite has loaded and defined the globals. So that's what you are seeing, it doesn't exist initially because Vite has not ran, then once it does, the globals are defined.

This is intended behavior, you should only rely on the globalThis behavior for scripts that are loaded via Vite, regular <script> with no define:vars should do what you want.

Also note that the globalThis behavior is dev-only so not really safe to use anyways.

@matthewp matthewp closed this as not planned Won't fix, can't repro, duplicate, stale Sep 17, 2024
@AriPerkkio
Copy link
Author

AriPerkkio commented Sep 17, 2024

The define:vars here was only used to highlight the issue, so here is our repro without it: https://stackblitz.com/~/edit/github-5kyhn9-b4rgq5?file=src/pages/index.astro

So basically:

<script>
  console.log("This is will cause crash", __MY_DEFINE__);
</script>

<script>
  setTimeout(() => console.log("This is defined", __MY_DEFINE__), 1_000);
</script>

So is it safe to assume that when accessing variables set by Vite's define in Astro, we should always use setTimeout to avoid them not being defined? This doesn't sound right to me.

@bluwy
Copy link
Member

bluwy commented Sep 17, 2024

It looks like it's because we injected the /@vite/client after the hoisted scripts in the head, which causes the problem. However, there's a workaround using experimental.directRenderScript config (which is enabled by default in Astro 5) that can workaround this so that the scripts are rendered at where it's declared instead (e.g. in the body) (among other benefits). Maybe this is enough for now? Stackblitz fork

We could maybe go back and fix this in v4 when experimental.directRenderScript isn't enabled, but perhaps not much value now.

@AriPerkkio
Copy link
Author

Thanks for the details, that makes sense. Having experimental.directRenderScript escape hatch for now works for us, and having that enabled by default in v5 will prevent others from running into this.

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

No branches or pull requests

3 participants