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

automatically add trailing slash against astroConfig.site #5604

Closed
1 task done
konojunya opened this issue Dec 14, 2022 · 3 comments · Fixed by #5608
Closed
1 task done

automatically add trailing slash against astroConfig.site #5604

konojunya opened this issue Dec 14, 2022 · 3 comments · Fixed by #5608
Labels
- P3: minor bug An edge case that only affects very specific usage (priority)

Comments

@konojunya
Copy link
Contributor

konojunya commented Dec 14, 2022

What version of astro are you using?

1.6.15

Are you using an SSR adapter? If so, which one?

None

What package manager are you using?

yarn

What operating system are you using?

Mac

Describe the Bug

In astro.config.mjs, the value specified in defineConfig is forced to be inserted with a trailing slash. The reason for this is that the transform of zod is used in core/config/schema.ts.

site: z
.string()
.url()
.optional()
.transform((val) => (val ? appendForwardSlash(val) : val)),

This state is not intended by the user, so that ogp, for example, requires an absolute path to be specified, so it is written as ${import.meta.env.SITE}/ogp.png, but the actual built HTML will contain https://example.com//ogp.png is written in the actual built HTML. Browsers will interpret this URL as convenient, but it may conflict with various other implementations.

After listening to the intent, the transform implementation of zod should be removed if it is not needed, and the URL should be left as specified by the user. If this is necessary, the docs should describe this behavior.

However, if you change it, you have to think about how to do it, or it will cause a bug if you simply stop using the trailing slash.

Link to Minimal Reproducible Example

https://github.com/konojunya/astro-import-meta-env-site

Participation

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

I'd like to hear from the PR author, @matthewp.

#4771

@matthewp
Copy link
Contributor

I don't think that was intentional. We don't use your site config for anything other than setting Astro.site, so you should be free to put any valid URL and we should not touch it. Will accept a PR.

@matthewp matthewp added the - P3: minor bug An edge case that only affects very specific usage (priority) label Dec 14, 2022
@konojunya konojunya changed the title forced trailing slash against astroConfig.site automatically add trailing slash against astroConfig.site Dec 14, 2022
@konojunya
Copy link
Contributor Author

@matthewp I have created a PR and would appreciate a REVIEW!

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)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants