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

Incorrect static file endpoint paths in dev (and prod build output) with trailingSlash: true (OP responded) #9674

Closed
1 task
firxworx opened this issue Jan 12, 2024 · 10 comments
Labels
- P4: important Violate documented behavior or significantly impacts performance (priority) pkg: astro Related to the core `astro` package (scope)

Comments

@firxworx
Copy link

firxworx commented Jan 12, 2024

Astro Info

Astro                    v4.1.2
Node                     v20.10.0
System                   Linux (x64)
Package Manager          pnpm
Output                   static
Adapter                  none
Integrations             @astrojs/tailwind
                         @astrojs/react
                         @astrojs/mdx
                         @astrojs/sitemap

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

No response

Describe the Bug

For an SSG site with trailingSlash: always in the config, the static file endpoints feature (see docs) should output files not directories in both dev server and production build.

Edit As clarified below in the comments:

  • the dev server generates a directory for static files e.g. what should be /pathname/to/file.json erroneously produces /pathname/to/file.json/ resulting in a disagreement between dev and production that will lead to bugs in projects
  • the astro build process generates the expected output in the dist folder (e.g. /pathname/to/file.json) however its console output reports an incorrect directory path similar to how the dev server behaves: /pathname/to/file.json/ (screenshot is below in the comments)

The trailingSlash: always functionality is crucial to preventing issues with links, etc. in projects because it enforces the requirements of production hosting environment during development and QA.

What's the expected result?

Refer to StackBlitz: https://stackblitz.com/edit/github-gxpxsn?file=astro.config.mjs

The static file generation feature is expected to work as-documented.

I should be able to use the feature to generate OG images, RSS feeds, robots.txt, and more on an Astro SSG project, regardless of what my trailingSlash and build.format settings are. Routing should work as expected.

In the StackBlitz I note that on my local project Astro is working as expected for static file generation for files in the base of src/pages but not in subfolders or with dynamic routing, but in the StackBlitz version there are also issues here too.

Link to Minimal Reproducible Example

https://stackblitz.com/edit/github-gxpxsn?file=astro.config.mjs

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 Jan 12, 2024
@lilnasy
Copy link
Contributor

lilnasy commented Jan 12, 2024

Regardless of whether traiklingSlash is default or "always", this is the output.
image

...which is what I'd expect given the source
image

Can you rephrase what the bug is?

@lilnasy lilnasy added needs response Issue needs response from OP and removed needs triage Issue needs to be triaged labels Jan 12, 2024
@firxworx
Copy link
Author

@lilnasy try writing code or just creating a link that points to that json file (per your example). you will need to add a slash on the end for it to not 404, which is useless and not reflective of production.

Related, the build output is incorrect as well:

image

This is actually why I thought that the endpoint paths were messed up in prod as well, my bad for trusting the build output and not checking the actual dist folder. Sorry for the confusion there.

The build output does not reflect the reality that you have observed, which is the expected result:

image

Either way there's definitely a bug when the dev server and the build process doesn't reflect production when it comes to static file endpoint paths, it arguably compromises its purpose as a dev server! As I noted the trailingSlash: true setting is important to reflect prod hosting setup.

@firxworx firxworx changed the title Incorrect static file endpoint paths in dev and prod with trailingSlash: true Incorrect static file endpoint paths in dev (and prod build output) with trailingSlash: true Jan 14, 2024
kidylee added a commit to kidylee/astro that referenced this issue Jan 15, 2024
kidylee added a commit to kidylee/astro that referenced this issue Jan 15, 2024
kidylee added a commit to kidylee/astro that referenced this issue Jan 15, 2024
kidylee added a commit to kidylee/astro that referenced this issue Jan 15, 2024
martrapp pushed a commit that referenced this issue Jan 15, 2024
@firxworx firxworx changed the title Incorrect static file endpoint paths in dev (and prod build output) with trailingSlash: true Incorrect static file endpoint paths in dev (and prod build output) with trailingSlash: true (OP responded) Jan 24, 2024
@natemoo-re natemoo-re added pkg: astro Related to the core `astro` package (scope) - P4: important Violate documented behavior or significantly impacts performance (priority) and removed needs response Issue needs response from OP labels Jan 24, 2024
@Fryuni
Copy link
Member

Fryuni commented Feb 3, 2024

I've just hit this while making Starlight work with SSR by prerendering all its routes.

With trailingSlash: 'always' the URL for hoisted scripts are injected with the trailing / after the extension, ending with .js/. Those scripts are generated correctly, but depending on the output mode and the adapter those links won't serve the file and instead return a 404, leading to a broken page.

@matthewp
Copy link
Contributor

If you want endpoints that do not have trailingSlash then trailingSlash: 'always' is probably not what you want, right? always does need to mean always.

@ematipico
Copy link
Member

I am going to close this one. It's not the first time users were "surprised" that trailingSlash was affecting endpoints too, especially the ones that emit images.

We will discuss if it's worth to allowlist "known" extensions, for provide an option.

Still, that's not a bug.

@ematipico ematipico closed this as not planned Won't fix, can't repro, duplicate, stale Feb 28, 2024
@firxworx
Copy link
Author

firxworx commented Mar 4, 2024

@matthewp + @ematipico imho the feature only makes sense when it applies to pages...

  • What use-case(s) would there be for having a trailing slash on file paths in development server only but not in the production build when trailingSlash: 'always' e.g. /posts/my-post/og.png/ vs. /posts/my-post/og.png
  • Why you don't you consider the discrepancy between the dev server behaviour (along with its console output) vs. the actual build output to be a bug??

If you had a strong case for the first point, I imagine you'd then agree that the second point is a bug because the discrepancy will inevitably lead to production issues with Astro sites (plus add a layer of complexity with creating conditional URL's depending on dev vs. production).


Why does the trailingSlash feature exist?

Per Astro docs https://docs.astro.build/en/reference/configuration-reference/#trailingslash (emphasis mine):

Use this configuration option if your production host has strict handling of how trailing slashes work or do not work.

Common real-world cloud deployment example

Suppose architecture of an AWS S3-hosted Astro project with CloudFront up-front. Many providers use this architecture under-the-hood as well as other AWS services (Amplify) and this example is transferable to other cloud providers.

One must use either an Edge Lambda or CloudFront Function to implement what Astro would call the trailing slash behaviour.

This is where one decides if URL's should end in a trailing slash or not, and how the site should behave when serving a request for a URL that either ends or does not end in a trailing slash.

When it comes to pages and a request comes in e.g. /about we need to rewrite the request to append /index.html OR .html depending on our chosen trailing slash behaviour, so that that request maps to an actual destination on S3 that was actually part of Astro's build output.

When it comes to files, i.e. requests with pathnames that end with a file extension, then we need to allow the request through.

For example: /posts/my-post/og.png because that's what the file will be on S3 per Astro build output (as it should).

This is why, imho, trailingSlash: 'always' should apply to pages and not files to fulfill its stated purpose per the docs. If it did, then the dev server's behaviour would actually agree with the Astro build output, and agree with realistic production host configuration "with strict handling of how trailing slashes work or do not work".

Since this is so common the AWS docs provide complete code example for both Edge Lambdas and CloudFront Functions.

e.g. add index.html to requests that don't include a file name: https://docs.aws.amazon.com/AmazonCloudFront/latest/DeveloperGuide/example-function-add-index.html

In production I use something more elaborate to check if a request is for a page vs. file vs. the naive example because dots are valid in filenames (I check for file types etc. and also handle MIME types correctly etc.) however the overall idea is exactly the same.

The general approach is also the same on both Astro SSG and SSR projects I have deployed.

Conclusion

If trailingSlash: 'always' worked the way I think it should (imho) then dev works like build/prod works like deployed to cloud provider (or even Apache2 on a shared web host!!).

It also means:

  • I don't have to write any special conditional logic to account for any discrepancies in URL's between dev and prod (e.g. /og.png/ vs. /og.png)
  • I can rest easy knowing that URL's and endpoints of my projects will behave the exact same way in production as on dev
  • Astro would prevent my team from making errors with URL's in dev that would break in production.
  • Any E2E tests will be reliable
  • Links within my project are consistent for scripting, SEO, etc.

At the very least, even if you disagree, maybe you can propose other features for Astro config that should be added?

@lilnasy
Copy link
Contributor

lilnasy commented Mar 4, 2024

Do you want something like trailingSlash: "pages", so that the dev server requires it but only for .astro pages?

@firxworx
Copy link
Author

firxworx commented Mar 4, 2024

@lilnasy that would be awesome and would be really appreciated!

I hope that the dev server vs. build output discrepancy also doesn't get lost in this shuffle.

@lilnasy
Copy link
Contributor

lilnasy commented Mar 4, 2024

You are right, Astro should make sure that dev behaves the same as production server. However, that becomes impossible when the production server is not an Astro SSR app.

There are too many static web servers with their own conventions. It's not realistic to have complete parity with them. The only thing we can offer is making our code do what the options say. And "pages" seems like it could be among them.

@firxworx
Copy link
Author

firxworx commented Mar 4, 2024

I appreciate the challenge you face here having worked on both Astro SSR and SSG projects!

I specifically chose the example of an OG image on an SSG blog to highlight the dev vs. prod discrepancy issue since this is such a popular usage for Astro, and a case where trailingSlash configuration has the potential to be valuable to reflect the deployment environment.

I agree with your standpoint that code should do what the options say and I really like your trailingSlash: 'pages' idea :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
- P4: important Violate documented behavior or significantly impacts performance (priority) pkg: astro Related to the core `astro` package (scope)
Projects
None yet
Development

No branches or pull requests

6 participants