Consistently pass the pathPrefix with linked files #3338
Conversation
|
Deploy preview for gatsbygram ready! Built with commit d94e0c271596de80a58788810cc5c07b04a5b02c |
|
I could also make it so that Let me know what you think. |
| pluginOptions = {} | ||
| ) => { | ||
| const defaults = { | ||
| ignoreFileExtensions: [`png`, `jpg`, `jpeg`, `bmp`, `tiff`], | ||
| pathPrefix, |
There was a problem hiding this comment.
This isn't necessary since pathPrefix is a global option and plugins shouldn't have option for overriding it.
There was a problem hiding this comment.
Sorry for the delay, I had to do some investigation into what other packages were doing.
When I remove this it fails, what should I be doing here instead? It's not clear how I should be using pathPrefix for options.
I was using remark-images for reference originally. Is this incorrect as well?
Should I be using globals like gatsby-link instead?
There was a problem hiding this comment.
What I'm saying is just pass pathPrefix to newLinkUrl instead of making pathPrefix an option.
| process.cwd(), | ||
| `public`, | ||
| validDestinationDir, | ||
| `/undefined-undefined.gif` |
There was a problem hiding this comment.
Why undefined
I believe the reason it's undefined is because when this is run, there are no actual images for it to the filenames from.
undefined-undefined.gif represents FILENAME-DIGEST.gif
I'm not sure where/when the filename and digest are created but at the time of the test, these values don't exist in the hash. I'm still looking into this.
It originates here but I'm not sure when those values actually get set (I'm still looking around).
For the added test itself
All we're doing here is taking the markdownAST children.
This is just validating that the additional pathPrefix is joined.
The existing test without the prefix is here: #L239-L258
I'm just adding an additional test to validate that the prefix exists when it's set.
| const newLinkURL = (linkNode, destinationDir, pathPrefix) => { | ||
| const linkPaths = [`/`, pathPrefix, destinationDir, newFileName(linkNode)] | ||
| .filter(function(lpath) { | ||
| if (lpath) return true |
There was a problem hiding this comment.
If you build with yarn develop; the urls are correct.
When building for production, the current code does not append the pathPrefix.
This just allows you to optionally specify a pathPrefix; it filters it out if pathPrefix happens to be empty or null.
There was a problem hiding this comment.
This produces the same result as joining paths like gatsby-link.
However, gatsby link has to create a string, run a regex and return a new string.
The logic I have just creates the string ignoring pathPrefix if it isn't set.
There was a problem hiding this comment.
ok, so you're filtering out the sometimes empty pathPrefix 👍
| @@ -76,7 +80,7 @@ module.exports = ( | |||
| // Prevent uneeded copying | |||
| if (linkPath === newFilePath) return | |||
|
|
|||
| const linkURL = newLinkURL(linkNode, options.destinationDir) | |||
| const linkURL = newLinkURL(linkNode, options.destinationDir, options.pathPrefix) | |||
There was a problem hiding this comment.
change options.pathPrefix to pathPrefix
|
Hey @seryl would you like to finish this out? |
|
Sorry I'd gotten sidetracked. I'll get those fixes in tomorrow.
|
|
Thanks! |
seryl
force-pushed
the
add-pathPrefix-support-remark-copy-linked-files
branch
from
d94e0c2 to
2e49112
Compare
|
Deploy preview for using-drupal ready! Built with commit 2e49112 |
Fixes copied gifs and svgs missing the pathPrefix for production builds.
Refs #2440