Creatiwity / gatsby-plugin-favicon

UNMAINTAINED - Favicon Gatsby plugin
MIT License
166 stars 27 forks source link

Incorrect URLs generated when pathPrefix value is set #48

Closed mikelax closed 5 years ago

mikelax commented 6 years ago

Overview

Using gatsby v2, I have a site with a value set for the pathPrefix parameter. Before I added this parameter, Both gatsby develop and gatsby build --prefix-paths worked correctly. Now I am seeing 404 errors trying to load icons in both development and when running the site after a build.

Here are example URLs that are trying to be loaded:

http://sites/somename/icons-e09e4c81dabea7f47e8e80e2478342f9/favicon-32x32.png http://sites/somename/icons-e09e4c81dabea7f47e8e80e2478342f9/favicon-16x16.png

Here are examples within the built HTML files:

<link rel="apple-touch-icon" sizes="32x32" href="//sites/somename/icons-e09e4c81dabea7f47e8e80e2478342f9/favicon-32x32.png" /> <link rel="apple-touch-icon" sizes="16x16" href="//sites/somename/icons-e09e4c81dabea7f47e8e80e2478342f9/favicon-16x16.png" />

You can see this is missing the hostname (and port when I was testing on port 8080 locally).

ycgarrido commented 6 years ago

image For me too

Vazerthon commented 6 years ago

I'm experiencing this problem as well.

It looks like the underlying webpack plugin has a prefix option - https://github.com/jantimon/favicons-webpack-plugin/blob/master/README.md#advanced-usage - and anything you put in your options in gatsby-config will be passed through to webpack. There might be a hacky way to get pathPrefix working with this but I haven't worked it out yet.

julien1619 commented 6 years ago

Indeed there is a weird behaviour in the favicons-webpack-plugin package with the prefix exposed by Webpack.

Here is the commit from my fork that could cause the issue while trying to fix it: https://github.com/Creatiwity/favicons-webpack-plugin/commit/f262255febeeca348bf7ae5cc140e0386c074e2c

Is there anything in your configuration that could explain the bug? I've tested it with a beta version of Gatsby, maybe the behaviour changed.

chasemccoy commented 6 years ago

I too am having this issue with a site using a path prefix 🙁

RomanHotsiy commented 5 years ago

@julien1619 the commit you are referring is indeed causing the bug:

if (publicPath.length === 0 || publicPath.substr(0) !== '/') {

publicPath.substr(0) is not the first symbol of publicPath. It's just a copy of publicPath. Use publicPath.substr(0, 1) or publicPath.charAt(0)

RomanHotsiy commented 5 years ago

Oh, looks like fixed the issue in https://github.com/Creatiwity/favicons-webpack-plugin/commit/0872de414061baaad0a2853c77ae75bc38b8bbbf

but package.json of this repo points to old commit:

https://github.com/Creatiwity/gatsby-plugin-favicon/blob/a2e579b2d96bfbd500fdb584abbb4b15697f5d16/package.json#L23

Could you update it?

julien1619 commented 5 years ago

@RomanGotsiy Yes I'll update today or tomorrow, thanks!

RomanHotsiy commented 5 years ago

Thanks!

@julien1619 would be great to update peerDependencies

    "gatsby": ">2.0.0-alpha", // I think should be >=2.0.0
    "react": "^16.4.2" // I think ^16 would be more appropriate
RomanHotsiy commented 5 years ago

Hey, @julien1619.

Just a kindly reminder 😊

julien1619 commented 5 years ago

3.1.6 is now published, thanks for the reminder!