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

fix: MDX cannot find relative image path without leading ./ #10705

Closed
wants to merge 1 commit into from
Closed

fix: MDX cannot find relative image path without leading ./ #10705

wants to merge 1 commit into from

Conversation

rishi-raj-jain
Copy link
Contributor

@rishi-raj-jain rishi-raj-jain commented Apr 6, 2024

Changes

fixes #10641

  • What does this change?

Ensures prefix if not present in the referenced image.

Testing

  • Added a test that optimizes a png and checks for presence of webp in the built project.

Docs

withastro/docs#7856

Copy link

changeset-bot bot commented Apr 6, 2024

🦋 Changeset detected

Latest commit: 40faa5a

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 Apr 6, 2024
Copy link
Contributor

@OliverSpeir OliverSpeir left a comment

Choose a reason for hiding this comment

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

Beautiful

@OliverSpeir
Copy link
Contributor

OliverSpeir commented Apr 8, 2024

I think we called this a minor when we did it for .md #9755

and I guess https://semver.org/ specifies that when you add functionality it becomes a minor, but this could be seen as simply fixing a bug where ./ was incorrectly required as the syntax without ./ should have always been valid

  1. MAJOR version when you make incompatible API changes
  2. MINOR version when you add functionality in a backward compatible manner
  3. PATCH version when you make backward compatible bug fixes

What do you think @rishi-raj-jain?

@rishi-raj-jain
Copy link
Contributor Author

@OliverSpeir

Appreciate the thorough review. Does make sense.

add functionality it becomes a minor

I think I still need to learn where to mark it as a minor / patch as sometimes I just end up confusing between them. The PR can be seen as fixing a bug because it's "expected" to be able to use the paths without slashes, or be seen as an added functionality from the creators perspective.

Copy link
Member

@ematipico ematipico left a comment

Choose a reason for hiding this comment

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

The PR can be seen as fixing a bug because it's "expected"

A good way to understand that is if the docs mention this feature. If not, than it's not a bug. In fact, why don't we update the docs to show the new feature?

@rishi-raj-jain I ask you the following tasks, if you don't mind:

  • update the test section of the template, and tell us how you tested the feature. Ideally, we want to add a new test
  • update the docs section of the template, and link the docs PR where we add a new example

@rishi-raj-jain
Copy link
Contributor Author

update the test section of the template, and tell us how you tested the feature. Ideally, we want to add a new test

added tests.

@rishi-raj-jain
Copy link
Contributor Author

update the docs section of the template, and link the docs PR where we add a new example

withastro/docs#7856

@github-actions github-actions bot added feat: markdown Related to Markdown (scope) pkg: svelte Related to Svelte (scope) pkg: vue Related to Vue (scope) pkg: example Related to an example package (scope) 🚨 action Modifies GitHub Actions pkg: react Related to React (scope) pkg: preact Related to Preact (scope) pkg: solid Related to Solid (scope) pkg: create-astro Related to the `create-astro` package (scope) pkg: astro Related to the core `astro` package (scope) pr: docs A PR that includes documentation for review labels Apr 11, 2024
@rishi-raj-jain rishi-raj-jain deleted the patch-1 branch April 16, 2024 09:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🚨 action Modifies GitHub Actions feat: markdown Related to Markdown (scope) pkg: astro Related to the core `astro` package (scope) pkg: create-astro Related to the `create-astro` package (scope) pkg: example Related to an example package (scope) pkg: integration Related to any renderer integration (scope) pkg: preact Related to Preact (scope) pkg: react Related to React (scope) pkg: solid Related to Solid (scope) pkg: svelte Related to Svelte (scope) pkg: vue Related to Vue (scope) pr: docs A PR that includes documentation for review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

MDX cannot find relative image path without leading ./
4 participants