-
Notifications
You must be signed in to change notification settings - Fork 10.3k
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
[gatsby-source-filesystem] add publicURL field #3669
Conversation
Deploy preview for gatsbygram ready! Built with commit eca5cb0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for tackling this! Looking good!
resolve: (file, fieldArgs, context) => { | ||
const details = getNodeAndSavePathDependency(file.id, context.path) | ||
|
||
const fileName = `${file.internal.contentDigest}${details.ext}` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing the .
between the two strings?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also let's include the original file name in the new pathname like here:
const imgSrc = `/${file.name}-${ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
details.ext
already contains .
, details.extension
(or something like that) doesn't contain it.
Anyway this for this and rest of your definitly valid comments - this is basicly copied https://github.com/gatsbyjs/gatsby/blob/master/packages/gatsby-transformer-sharp/src/extend-node-type.js#L105 minus image width, height part, plus path prefix which is propably missing in code I linked, actual image processing methods (sizes, resolutions) do honor path prefix.
I will fix those issues propably tomorrow.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh haha, well please fix transformer-sharp while you're at it :-)
if (!fs.existsSync(publicPath)) { | ||
fs.copy(details.absolutePath, publicPath, err => { | ||
if (err) { | ||
console.error(`error copying file`, err) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add the origin path and public path
const publicPath = path.join( | ||
process.cwd(), | ||
`public`, | ||
`static/${fileName}` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
make fileName
another argument instead of joining them here.
…rectory and getting their paths
…error in ImageSharp.original field, honor path prefix when returning public url, use image name as part of image created in static directory
Updated code. Also added gatsby-transformer-sharp changes here in seperate commit to not create more spam with seperate PR - if needed i can split it to keep it more contained. |
Thanks! Now I can finally stop asking people in discord to implement this to solve their problem 😅 |
* add publicURL field to File node to allow copying assets to static directory and getting their paths * [gatsby-transformer-sharp] add more details when reporting file copy error in ImageSharp.original field, honor path prefix when returning public url, use image name as part of image created in static directory * Format
This allow files to get copied to
static
directory and get their urls.Closes #3538