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

[@astrojs/image] Handle missing trailing slash in processStaticImage #6421

Merged
merged 1 commit into from
Mar 7, 2023
Merged

[@astrojs/image] Handle missing trailing slash in processStaticImage #6421

merged 1 commit into from
Mar 7, 2023

Conversation

alixinne
Copy link
Contributor

@alixinne alixinne commented Mar 4, 2023

Changes

The code path changed by this commit isn't only taken when running using Vite. If the site is configured with a base url which is different from / but does not end with / (for example, because trailingSlash is set to never), the - 1 results in an off-by-one error when truncating the URL.

By checking if the base url ends with /, we can determine the right length for the prefix to truncate.

Testing

Tested manually on a repository which exhibited the issue:

  • With config.base == '/', before change: working
  • With config.base == '/pr-preview/pr-5', before change: error messages in the log indicate that the integration is looking for files beginning with 5/path/to/the/image
  • With config.base == '/', after change: working
  • With config.base == '/pr-preview/pr-5': after change: error messages are gone and images are generated as expected.

Docs

No docs added since this is only a bug fix for a behavior that should have already been working.

The code path changed by this commit isn't only taken when running using Vite. If the site is configured with a base url which is different from `/` but does **not** end with `/` (for example, because `trailingSlash` is set to `never`), the `- 1` results in an off-by-one error when truncating the URL.

By checking if the base url ends with `/`, we can determine the right length for the prefix to truncate.
@changeset-bot
Copy link

changeset-bot bot commented Mar 4, 2023

🦋 Changeset detected

Latest commit: 67a8800

The changes in this PR will be included in the next version bump.

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@github-actions github-actions bot added the pkg: integration Related to any renderer integration (scope) label Mar 4, 2023
@matthewp matthewp merged commit e58a925 into withastro:main Mar 7, 2023
@astrobot-houston astrobot-houston mentioned this pull request Mar 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pkg: integration Related to any renderer integration (scope)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants