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

[Content] Throw on relative image usage #5648

Merged
merged 10 commits into from Dec 22, 2022

Conversation

bholmesdev
Copy link
Contributor

Changes

We decided relative image resolution was out-of-scope for the Content Collections RFC. Throw on relative image paths in src/content/ for Markdown and MDX.

Testing

Manual testing

Docs

N/A - Already documented that Markdown and MDX are the only allowable files.

@changeset-bot
Copy link

changeset-bot bot commented Dec 19, 2022

🦋 Changeset detected

Latest commit: c2f2435

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 pkg: astro Related to the core `astro` package (scope) pkg: integration Related to any renderer integration (scope) feat: markdown Related to Markdown (scope) labels Dec 19, 2022
packages/integrations/mdx/src/plugins.ts Outdated Show resolved Hide resolved
pnpm-lock.yaml Outdated Show resolved Hide resolved
@andersk
Copy link
Contributor

andersk commented Dec 20, 2022

Can this check be optional? Right now relative paths are the only way to reference images from *.md that works with arbitrary values of BASE_URL. It’s not ideal that they’re relative to the output URL rather than the input file, but better than impossible.

Copy link
Member

@natemoo-re natemoo-re left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@natemoo-re
Copy link
Member

Can this check be optional? Right now relative paths are the only way to reference images from *.md that works with arbitrary values of BASE_URL. It’s not ideal that they’re relative to the output URL rather than the input file, but better than impossible.

I think unfortunately, this is why this setting can't be optional. We're trying to protect against anyone from relying on the current unspecified behavior. The eventual plan is to specify the correct behavior for relative paths, which likely means that they'll be resolved and treated as actual assets. That would also solve the BASE_URL issue, but have lots of other benefits as well.

@bholmesdev bholmesdev force-pushed the fix/content-collections-rel-image-error branch from ae93337 to c2f2435 Compare December 22, 2022 15:10
@bholmesdev bholmesdev merged commit 853081d into main Dec 22, 2022
@bholmesdev bholmesdev deleted the fix/content-collections-rel-image-error branch December 22, 2022 15:38
@astrobot-houston astrobot-houston mentioned this pull request Dec 22, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feat: markdown Related to Markdown (scope) pkg: astro Related to the core `astro` package (scope) pkg: integration Related to any renderer integration (scope)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants