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(gatsby-remark-copy-linked-files): respect assetPrefix #26976

Merged
merged 3 commits into from
Sep 21, 2020

Conversation

pieh
Copy link
Contributor

@pieh pieh commented Sep 21, 2020

Description

Function that generates urls in gatsby-remark-copy-linked-files is not compatible with assetPrefix - current path joining result in something like /https:/cdn.your-domain.com/{generated-hash-values}/demo.svg (note that first character in front of protocol part).

This PR changes to URL generation to match what gatsby-plugin-sharp already do (which is just doing string manipulation, because pathPrefix is already passing in form that allows for it - it strips trailing slash always, which means that using ${prefix}/some/path will work correctly):

const prefixedSrc =
(pathPrefix ? pathPrefix : ``) +
`/static/${digestDirPrefix}` +
encodedImgSrc

Related Issues

Fixes #25918

@gatsbot gatsbot bot added the status: triage needed Issue or pull request that need to be triaged and assigned to a reviewer label Sep 21, 2020
@pieh pieh added topic: remark/mdx Related to Markdown, remark & MDX ecosystem type: bug An issue or pull request relating to a bug in Gatsby and removed status: triage needed Issue or pull request that need to be triaged and assigned to a reviewer labels Sep 21, 2020
Comment on lines -310 to +402
`/undefined/undefined.gif`
fileLocationPart
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this and other adjustments are not necessary for this PR, but as I looked to add cases for assetPrefix I noticed those undefined there in expecteted strings and thought that it doesn't inspire a lot of confidence in those tests so I adjusted tests to specify internal.contentDigest and name fields on File nodes so those are more specific assertions

@gatsby-cloud
Copy link

gatsby-cloud bot commented Sep 21, 2020

Gatsby Cloud Build Report

gatsby

🎉 Your build was successful! See the Deploy preview here.

Build Details

View the build logs here.

🕐 Build time: 21m

Copy link
Contributor

@pvdz pvdz left a comment

Choose a reason for hiding this comment

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

lgtm

@pieh pieh added the bot: merge on green Gatsbot will merge these PRs automatically when all tests passes label Sep 21, 2020
@gatsbybot gatsbybot merged commit 6270c3d into master Sep 21, 2020
@delete-merged-branch delete-merged-branch bot deleted the fix/grclf-asset-prefix branch September 21, 2020 15:10
@wardpeet
Copy link
Contributor

Successfully published:

  • gatsby-remark-copy-linked-files@2.3.16

pragmaticpat pushed a commit to pragmaticpat/gatsby that referenced this pull request Apr 28, 2022
…6976)

* test(gatsby-remarkc-copy-linked-files): get rid of "undefined" in assertions - it doesn't inspire confidence

* fix(gatsby-remark-copy-linked-files): honor assetPrefix correctly

Co-authored-by: gatsbybot <mathews.kyle+gatsbybot@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bot: merge on green Gatsbot will merge these PRs automatically when all tests passes topic: remark/mdx Related to Markdown, remark & MDX ecosystem type: bug An issue or pull request relating to a bug in Gatsby
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Asset Prefix does not works on SVG
5 participants