nfl / react-helmet

A document head manager for React
MIT License
17.37k stars 661 forks source link

can not overwrite favicon #430

Open HuangQiii opened 5 years ago

HuangQiii commented 5 years ago

(May be a) Bug report

What is the current behavior?

when i use favicon link in helmet, the origin favicon can not be overwrite.

If the current behavior is a bug, please provide the steps to reproduce.

<Helmet preserved>
          {title}
          <link rel="shortcut icon" href="https://avatars3.githubusercontent.com/u/11797760?s=460&v=4" />
        </Helmet>

image

What is the expected behavior?

i think the favicon will be overwrite

what i do now

i use a window function to solve it temporary,

window.updateFavicon = function(faviconUrl) {
        var link = document.querySelector("link[rel*='icon']") || document.createElement('link');
        link.type = 'image/x-icon';
        link.rel = 'shortcut icon';
        link.href = faviconUrl;
        document.getElementsByTagName('head')[0].appendChild(link);
      }

all maybe there are other consider of this question, please tell me

thx😄

Joelkang commented 5 years ago

We found this same issue and forked the library, providing a helmetKey to inform helmet to override any previous tags with the same key: https://github.com/Frameio/react-helmet

tmbtech commented 5 years ago

@joelkang would if be possible to provide a PR? Maybe we can bring it into the project?

foreseaz commented 5 years ago

@HuangQiii hey there, for Favicon overwrite, the original favicon tag should be removed first and then attach the alternative one. An example of manipulating favicon is here, feel free fork one to use with react-helmet https://github.com/foreseaz/react-loadcon

biber-zoli commented 5 years ago

Hiya, may I ask if this bugfix is in the pipeline? We'd also like to dynamically update the favicon and the title, it's a pity this react-helmet favicon bug is tripping us up... Thx

kmjennison commented 5 years ago

Here's a workaround using the onChangeClientState prop which removes existing link elements with rel="icon". You'll have to modify this example if you use other rel values or the sizes attribute.

import { filter } from 'lodash/collection'
import { get } from 'lodash/object'
import { Helmet } from 'react-helmet'

<Helmet
  // Handle a react-helmet bug that doesn't replace or remove
  // existing favicon <link /> elements when a new favicon is set.
  // https://github.com/nfl/react-helmet/issues/430
  // If we add a new favicon link element, remove all other favicon
  // link elements and re-add the new favicon link element.
  onChangeClientState={(newState, addedTags) => {
    try {
      // Check if we added any new link[rel="icon"] elements.
      const newFaviconElems = filter(get(addedTags, 'linkTags', []), {
        rel: 'icon',
      })
      if (!newFaviconElems.length) {
        return
      }

      // Get the href of the last link element we just added, which
      // we assume is the most recently-added favicon.
      const newFaviconHref =
        newFaviconElems[newFaviconElems.length - 1].href

      // Remove all react-helmet link elements with rel="icon" that
      // do not have the href of our recently-added favicon.
      // If no other favicon elements exist, we don't have to do
      // anything.
      const existingFavicons = document.querySelectorAll(
        `link[rel="icon"][data-react-helmet]:not([href="${newFaviconHref}"])`
      )
      if (!existingFavicons.length) {
        return
      }
      existingFavicons.forEach(e => e.parentNode.removeChild(e))

      // Remove and re-add the latest link element to make sure
      // the browser uses it.
      const newFaviconElem = document.querySelector(
        `link[rel="icon"][data-react-helmet][href="${newFaviconHref}"]`
      )
      if (newFaviconElem) {
        const parentElem = newFaviconElem.parentNode
        parentElem.removeChild(newFaviconElem)
        parentElem.appendChild(newFaviconElem)
      }
    } catch (e) {
      console.error(e)
    }
  }}
/>

Would love to see this bug fixed, though.

douglasbouttell commented 5 years ago

Having had a look through the code, I have came up with my own solution/work-around.

react-helmet will do a isEqualNode (MDN Link) on new elements it creates, but since it adds data-react-helmet=true your existing element needs to have it.

For example:

In your index.html

<!DOCTYPE html>
<html lang="en">
  <head>
    <link id="favicon" rel="icon" href="/favicon.ico" type="image/x-icon" data-react-helmet="true" />
    <!-- ... -->
  <head>
  <!-- ... -->
</html>

and in your react component

<Helmet>
  <link id="favicon" rel="icon" href="/new-favicon.ico" type="image/x-icon"/>
</Helmet>