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 preview fails silently with missing dist folder #9297

Closed
1 task
hirasso opened this issue Dec 4, 2023 · 10 comments · Fixed by #9544
Closed
1 task

astro preview fails silently with missing dist folder #9297

hirasso opened this issue Dec 4, 2023 · 10 comments · Fixed by #9544
Labels
- P2: nice to have Not breaking anything but nice to have (priority) help wanted Please help with this issue!

Comments

@hirasso
Copy link
Contributor

hirasso commented Dec 4, 2023

Astro Info

Astro                    v3.6.4
Node                     v18.19.0
System                   macOS (x64)
Package Manager          npm
Output                   static
Adapter                  none
Integrations             astro-expressive-code
                         @astrojs/mdx

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

No response

Describe the Bug

Hi there! I'm really not sure if this is a problem with Astro, PlayWright's webServer or GitHub actions 😅

I have a little package where I create the docs with Astro and also use the same docs to test my package through PlayWright (see the config). This is what I tried first for PlayWright's webServer.command (contrived example, actually the commands are in npm scripts to make sure they are being run from the root of my package):

astro preview --root docs

It was working nicely locally, but as soon as I put it in a GitHub action, playWright was hanging indefinitely.

What's the expected result?

The PlayWright tests should be able to resolve the provided server (created by astro preview)

Turns out first building Astro before serving the preview fixed my issue in CI:

astro build --root docs && astro preview --root docs

Now I'm curious: What's the difference here? Doesn't astro preview also build the astro site?

There is a related thread over at PlayWright and I'll happily close this issue if the astro maintainers think this is an issue with PlayWright. But then I'd still love to learn why combining astro build and astro preview isn't the same as astro preview alone.

Link to Minimal Reproducible Example

n/a

Participation

  • I am willing to submit a pull request for this issue.
@github-actions github-actions bot added the needs triage Issue needs to be triaged label Dec 4, 2023
@hirasso
Copy link
Contributor Author

hirasso commented Dec 4, 2023

Does a minimal reproducible example make sense here? I'd say my package is as minimal as it gets 😄. It contains an astro site, a playwright test suite and a github action to run the tests.

@lilnasy
Copy link
Contributor

lilnasy commented Dec 4, 2023

This seems like a general question, so a reproduction is not necessary. Although our discord server is where queries similar to yours would get a faster response: https://astro.build/chat.

To answer the question: no, astro preview does not run build, it simply runs a static file server for whatever it finds in the "dist" folder, which in this case was nothing. Relevant docs.

@lilnasy lilnasy closed this as not planned Won't fix, can't repro, duplicate, stale Dec 4, 2023
@hirasso
Copy link
Contributor Author

hirasso commented Dec 4, 2023

Oh my god. That's why it was working locally. The build folder was already there 🤪 Thanks for the quick reply!

@hirasso
Copy link
Contributor Author

hirasso commented Dec 4, 2023

...revisiting the docs I must confess that it didn't make it fully clear to me that astro preview wouldn't build also. Maybe a little Hint box would be helpful here for others as well? I'll create a PR to the docs tomorrow. Thanks again for the help!

@lilnasy
Copy link
Contributor

lilnasy commented Dec 4, 2023

Awesome! Thanks for the help.

@hirasso
Copy link
Contributor Author

hirasso commented Dec 5, 2023

I just tested running astro preview without first building. This is the output in the console (no warning about a missing dist folder!):

❯ npx astro preview --root docs
  🚀  astro  v3.6.4 started in 14ms
  
  ┃ Local    http://localhost:8274/
  ┃ Network  use --host to expose

...and in the browser I get a 404:

CleanShot 2023-12-05 at 09 43 04@2x

@lilnasy I think some kind of warning in the console (or even in the browser?) would be helpful here. If you agree, should I create a feature request?

@hirasso hirasso changed the title astro preview breaks PlayWright webServer in CI astro preview not warning about missing dist folder Dec 5, 2023
@hirasso hirasso changed the title astro preview not warning about missing dist folder astro preview fails silently with missing dist folder Dec 5, 2023
@lilnasy lilnasy reopened this Dec 5, 2023
@lilnasy
Copy link
Contributor

lilnasy commented Dec 5, 2023

Yes, this is something new users frequently encounter. So if you contribute a warning, it will really help!

We are planning to show the list of all routes on the 404 page in the dev server, we will probably tackle the same for preview server as well. For now, a warning about dist being empty would be valuable.

@lilnasy lilnasy added - P2: nice to have Not breaking anything but nice to have (priority) and removed needs triage Issue needs to be triaged labels Dec 5, 2023
@rishi-raj-jain
Copy link
Contributor

rishi-raj-jain commented Dec 5, 2023

I just tested running astro preview without first building. This is the output in the console (no warning about a missing dist folder!):

❯ npx astro preview --root docs
  🚀  astro  v3.6.4 started in 14ms
  
  ┃ Local    http://localhost:8274/
  ┃ Network  use --host to expose

...and in the browser I get a 404:

CleanShot 2023-12-05 at 09 43 04@2x

@lilnasy I think some kind of warning in the console (or even in the browser?) would be helpful here. If you agree, should I create a feature request?

Do we want to show a warning here or a no preview message because no dist (like we do for adapters like Vercel), @lilnasy?

@bluwy
Copy link
Member

bluwy commented Dec 7, 2023

I think a warning/message that bails like Vercel (because it doesn't support the preview command) works for me. So technically speaking, around this code:

if (settings.config.output === 'static') {
const server = await createStaticPreviewServer(settings, logger);
return server;
}
if (!settings.adapter) {
throw new Error(`[preview] No adapter found.`);
}
if (!settings.adapter.previewEntrypoint) {
throw new Error(
`[preview] The ${settings.adapter.name} adapter does not support the preview command.`
);
}

We could also simply throw an error for consistency. I don't think there's a case where we still want to start the preview server.

@bluwy bluwy added the help wanted Please help with this issue! label Dec 7, 2023
@hirasso
Copy link
Contributor Author

hirasso commented Dec 7, 2023

I'd also like it to fail with an error.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
- P2: nice to have Not breaking anything but nice to have (priority) help wanted Please help with this issue!
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants