fix(gatsby-plugin-manifest): fixes icons not getting asset prefix

[moonmeister]

Description

Modifies any path provided for a manifest icon to be prefixed with appropriate path or asset prefix.

Currently this affects all provided paths including those from manual, hybrid, and automatic modes. I think this makes the most sense, not sure if there will be any edge cases that need a work around.

This change could potentially create a breaking change for people using asset prefix in manual/hybrid mode who have provided full paths to work around the plugin not handling this for them,.

Documentation

I haven't written any, Do we think we should document this change in behavior?

Related Issues

Fixes: #18497

[freiksenet]

Some end-to-end tests break on ASSET_PREFIX, I wonder if you could fix them.

[moonmeister]

Some end-to-end tests break on ASSET_PREFIX, I wonder if you could fix them.

So what's the fix? Do i just need to set an empty string if the global isn't already set? If that's the case it seems like this should be handled by gatsby-link? That way every place that uses this doesn't have to check for an non-existent global? Or should the config handler be setting __PATH_PREFIX__ global no matter whether there is something in my gatsby-config? It looks like I could also use the modules from the API, would that be a fix?

What's the "correct" path here? Thanks.

[wardpeet]

withPrefix helpers are only accessible in webpack land so gatsby-ssr & gatsby-browser.

In gatsby node you'll have to use the basePath property that is given to you by onPostBootstrap. You only need to fix the src urls inside manifest.webmanifest, not while generating the files.

exports.onPostBootstrap = async (
  { reporter, parentSpan, basePath },
  { localize, ...manifest }
) => {
// ...
await makeManifest(cache, reporter, manifest, false, basePath)

[moonmeister]

@wardpeet Thanks for the info, reverted my earlier changes and redid them, everything seems much easier now. From my testing this doesn't do "assetPrefix"

• What am I missing to prefix the assetPrefix?
• What do you think about applying this to the start_url option in the manifest as well?

Thanks.

[wardpeet]

What am I missing to prefix the assetPrefix?

I'll check this out in a bit.

What do you think about applying this to the start_url option in the manifest as well?

That would be great! If you could do that.

[ehannes]

Any news on this @moonmeister?

[moonmeister]

Think I was waiting on @wardpeet

[wardpeet]

My bad, I flagged it to check this out tomorrow morning. I'm really sorry to keep you waiting

[wardpeet]

i've added tests and fixed a small bug with start_url. It should all be operational now and working. I'll merge on monday, thanks @moonmeister for your work! 🙏

[wardpeet]

Everything is green, let's do this!