-
Notifications
You must be signed in to change notification settings - Fork 661
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
any possible way to disable 'data-react-helmet="true"' from server side rendering? #79
Comments
We currently use that markup to distinguish Helmet's tags from existing tags in the app, in case a user wants tags managed outside of Helmet. The current implementation clears all the tags managed by Helmet and re-adds them. If we eliminated this from the server, we couldn't distinguish on the front end. We will be moving to an implementation that will track individual tags on the front end and only update the changed ones. But for that we still need the client to establish a set of known Helmet-managed tags. We could potentially re-visit why we have separate Helmet-managed tags and consider having Helmet manage everything...or perhaps an opt-in for full management vs. partial management. Hmmm....thoughts? |
@cwelch5 Having such an option would be cool |
Still nothing? |
Realy need to disable data-attributes from meta tags on server side. I have asked support of Yandex and they say data attributes not allowed in meta tags |
@dergachevm As a workaround, you can replace it with empty string. Something like: const Head = Helmet.rewind()
const regexp = / data-react-helmet="true"/g
const attr = Head.htmlAttributes.toString()
const title = Head.title.toString().replace(regexp, '')
const link = Head.link.toString().replace(regexp, '')
const style = Head.style.toString().replace(regexp, '')
const meta = Head.meta.toString().replace(regexp, '')
const script = Head.script.toString().replace(regexp, '') |
@svagi Thank you |
Anyways I can do this in the client? I have a React client side rendered page and trying to use meta tags for Twitter Card. |
Only remove the data attribute from element sets that aren't already in the
Otherwise, the |
@cwelch5 Can you make In the mean time, a dirty workaround is: require('react-helmet/lib/HelmetConstants.js').HELMET_ATTRIBUTE='data-react'; Personally, I don't feel comfortable exposing the fact that the server-side code is using a particular framework. Exposing in markup that |
I am wondering if anybody has evidence that the We are using Our SEO consultant suspect that the attribute Since we are implementing an isomorphic application, removing the So I am considering working on a fork (or pull-request) which does not use the attribute |
I've run into another issue with this data attribute — it breaks Google AMP's [albeit strict] validation for their boilerplate tags. |
Please. 🙏 |
+1 This is affecting Google's AMP pages |
It also affect Facebook debugger |
In order to deal with this annoying behaviour, you can do this hack, which removes the
|
It affects the Facebook open graph... |
Same problem here for Twitter Cards |
any updates on this one? |
I have forked react-helmet and removed the There is one caveat, if you include any metadata in your render using alternative methods (not react-helmet syntax), you must assign the HTML element the attribute Obviously it would be preferable if react-helmet included an option to disable use of the attribute, but given this issue is over 2 years old that seems unlikely. As a side note, the fork was originally created to enable support of the react 16 server-side streaming interface. |
Any updates on this? |
Any news on this? |
|
Our project is using Helmet too and we use server side rendering. When checking the SEO with https://seositecheckup.com/ we get missing tags (especially the description meta tag) although it is there. We suspect that the |
How to add meta tag of twitter card in react-helmet to render dynamic data |
+1 |
all these "problems" are not due to react-helmet.... One of my personal favorites is as @gajus said
Wrong in so many cases, usage of react-helmet is negligible. Serve it as cold html ... is that html a vector?... anyway even then you can remove it from the generated static html files. (your app js as react is still recognizable as react.. still can see anything you use... usage of react-helmet is negligible); If you don't like the attribute, the fear of the dark; The only reason for removing the attribute is management purity. Which in most cases is negligible. If you think you may need to remove it then add a job after the build. |
@AubreyHewes - it's not very constructive to tell people that there's no problem in helmet. This has been a long-standing issue and lots of people are clearly having issues with this, especially in the realm of unfurling (which is why I'm here), so please don't brush it off. Not everybody has the skills or ability to write I suggest that instead of writing arbitrary attributes to the helmet-controlled nodes, helmet should precede all helmet nodes with a special/configurable html comment: <head>
<!-- helmet -->
<meta property="og:title" description="..." />
</head> Then helmet could do something like this to get all of its controlled nodes: const meta = Array.from(document.head.querySelectorAll('meta')).map(meta =>
meta.previousSibling.nodeType === 8 && meta.previousSibling.textContent.trim() === 'helmet'
) Not so hacky solution, but with caveatsFor those who want to try out a post build script, you can do the following. Please note that this will fix the server-generated markup, but the mounted SPA will result in duplicate tags because helmet cannot differentiate between tags it created and tags it did not. This will not break your app, but it might confuse bots which execute javascript.
|
@DesignByOnyx your solution is exactly what I said
I was not brushing anything off. Your idea of a prepended comment does not work in react-helmet; and just exposes the exact same information (the original issue). But still, when a developer uses this library they should know what they are doing, i.e. accept this issue /or resolve it themselves. The solution for static generated sites is to remove the attribute (as I stated and your solution). Non static sites are crappy seo anyway. If you are not static but public.. then you need to rethink your application decisions. This is only a problem if you think it is a problem or do not like the attribute. I have yet to generate a static site that does not work properly with opengraph compatible consumers (fb/twitter/etc -- unfurling). |
I'm not doing SSR and I am trying to load a 3rd party script and it's complaining about the attribute @AubreyHewes what is the problem with just adding a prop to get rid of this attr? |
@scragg0x As you stated "Now I fully understand that the 3rd party script should just ignore it"... Your best chance is to reach out to the 3rd party that they should ignore things that have nothing to do with them. Or use a 3rd party lib that does not test things that are not their responsibility... (which lib?) To remove the prop requires ... well look at the codebase. |
@AubreyHewes The library is from a CC payment gateway service, not an open source project on github so it's unlikely to change in a timely manner. I am resorting to a vanilla JS approach to loading the script. I use |
Same situation here, for example Google site verification also fails because of this, so I rely on hardcoding it in the index.html instead. I had issues with other platforms too, like Facebook, especially when it's about verification. It would be really nice to have an option (a prop?) to disable it. |
The current solution is to add a post build script that scrubs the attribute as previously proposed. Though this depends on if the app/site in question is static SPA vs dynamic SPA.
For a possible postbuild script see #79 (comment) @scragg0x So you are deploying as a dynamic SPA? If so there is not really a solution... the dynamically generated html will always have the attribute. To reiterate this is not actually really a problem with the library but the usage/expectation within other frameworks. |
Using https://github.com/ds300/patch-package and replacing "data-react-helmet" for "data-react" in node_modules did the trick for me. It's hacky but not that much ™️ |
This ticket was opened in 2015.. clearly people still want the ability to remove this. Whether its a bug, an enhancement, or a new feature, is not the point. The community around this open-sourced project want the ability to disable this attribute. In the worst case, maybe there is a bug in some use-cases that you're not aware of. In the best case, its people complaining about semantics of if attributes should be included on the meta tag. Regardless, of the situation, its something people are coming to this ticket about because they want the ability to disable this. This seems reasonable to me and after 6 years I don't see why its been pushed into a corner for so long. I don't think this should continue to be ignored. |
still there is no decent solution to this? |
Bumping this again because maintainers behaving this way really gets to me. Let me take a stab on some next steps we could take: @AubreyHewes I see PRs posted above long ago, and some discussion around your feeling that this behavior (which is evidently not a bug) is best solved by external libraries and post-processing. To me, that sounds very brittle and incredibly opinionated for a library that should ostensibly be quite _un_opinionated. Not to mention, many people here find this behavior problematic for one reason or another; the reason itself does not really matter because the concerns are valid. So if you don't like the above solutions @AubreyHewes, would you please take a moment to let us know what a feasible solution might look like? What are your concerns, and how can we turn those into constraints to meet your expectations as a maintainer here? Maybe then we can put up an appropriate PR and get this behavior changed for the Side note The way maintainers are handling this issue is pretty sad, and I'm sorry @AubreyHewes but your responses here (may I presume you're a maintainer?) are just plain unprofessional and against the spirit of open source. There's no point in you telling others what solutions their use cases mandate, or whether their problems are valid.
That's like telling a sick person to just 🪄_imagine_✨ they're not sick anymore, then boom, they're not sick anymore. Clearly a critical mass of people a) believes this functionality is a problem; and/or b) does not like the attribute. Therefore it a) is a problem, and b) might be too opinionated for the purpose of this library. I'm personally not in a position to contribute but I'd bet many people are. Maintainers, please (on behalf of your community) either fix this or coordinate with contributors to fix it. That's the entire point of being a maintainer of a popular library. Stewarding the library and facilitating the community's contributions. |
@devinhalladay I am not a maintainer. Your issues are with a perceived issue of using this library. I have only ever proposed and described working solutions that possibly alleviate those issues. Possibly a bandage, but a working bandage without rewriting the complete core. That is also open source. "To reiterate this is not actually really a problem with the library but the usage/expectation within other frameworks." If you yourself have a simple solution please PR it, you will help all these people having this problem as well as yourself. That is open source. A question I actually have for you is why you are still using this library? Most frameworks lift this work themselves.. So what are you using and what entailed such a detailed but wrongly attributed/detailed comment? Side note: If you are not able to contribute to a library then there is no reason to explain how you can only complain |
Thank you @AubreyHewes that's clarifying and very valid! Apologies for coming in a bit hot. |
react helmet is not for seo |
Reading over this recent conversation that @devinhalladay and @AubreyHewes had (and the lack of any further notes from the maintainers) seems to indicate that this project might not be actively maintained as before? Is that the case? |
the react helmet attribute does not work with twitter cards. Twitter is one of the biggest social media platform out there. And your team doesn't want to add a native way to disable them? |
@KneePham See my comment just above yours. This library looks to be dead. |
The problem is that this library literally cannot work if the attributes are removed (and I don't use the word "literally" very often). I've looked at the code and tried several tricks to get around this... to no avail. The authors are not going to repeat themselves again. It's not that they're unwilling/unable to change it - it's that they can't. Your best bet is to use a post-build script to go through and remove the attributes from the compiled code. I provided examples of how to do this above. |
@KneePham Did you check if you have the correct Twitter Cards should work with the |
I decided to give a test by creating a empty index.html file with just the twitter meta tags. Apparently it works with any sort of attributes including in the twitter meta tags when testing with the twitter validator. Including data-react-helmet="true". So the issue might not be the attribute... but something entirely different? There is definitely something weird going wrong with how the meta tags are being render with this library. or a combination with this library + react. *Yes, I tried with the right content-type. Still doesn't work. |
Yup. It's not the |
I need to help :( |
I think it's because this react plugin changes the index.html only if js is enabled... I suppose that the social engines that generates the preview reads only the "crude" index.js generated in the build process. To bypass this trouble i've created this usefull npx command: https://github.com/Elius94/seo-injector Enjoy |
Hi, I am new in Gatsby. In what file you foresee to write this code. Thanks in advace |
Any progress on this? |
i don't mind if its in the client but i don't like these extra chars in my markup.
any suggestions?
thanks
The text was updated successfully, but these errors were encountered: