Coder Social home page Coder Social logo

Comments (14)

tizmagik avatar tizmagik commented on September 23, 2024 3

types may be called something like own or defined? Idk^^

Yea, agreed, β€œtypes” is kind of a weird name. Open to other suggestions as well...

from react-head.

JoshuaToenyes avatar JoshuaToenyes commented on September 23, 2024 1

Thank you for pointing that out @CanRau. I revised my opinion on Option 2 above :)

from react-head.

tizmagik avatar tizmagik commented on September 23, 2024

I think I'm a fan of Option 2. What are your thoughts @CanRau @TrySound @rdsedmundo @TrejoCode @JoshuaToenyes ? Any other options you can think of? Would love your thoughts!

EDIT: I added an Option 4 above, the nuclear option πŸ’£ 😁

from react-head.

TrejoCode avatar TrejoCode commented on September 23, 2024

For my use, option 2 favors me and in general, it seems like a simple solution to implement.

from react-head.

JoshuaToenyes avatar JoshuaToenyes commented on September 23, 2024

Very nicely described. Here are my thoughts on each option:

Option 1 - Manually created whitelist

Specifically on the "cons" list:

  • I don't think the whitelist syntax ([foo^=bar]) would be too complicated, and there are loads of resources that could be linked to for the unfamiliar.
  • A shared constant between client/server implementation could be used for the whitelist (we do this very frequently).
  • For solving Problem 2, seems like if I set whitelist={['title']} then any existing duplicate <title> tag would be clobbered on the client when rendered, right?

In general, I like the explicit approach Option 1 provides.

Option 2 - Auto-Whitelist

I can see how this would be attractive... but honestly I much prefer the data-rh attributes over adding a whole new script tag with duplicate title tags. Even if the script tag was minimized using the first few characters, I still would prefer the data-rh approach.

I agree that data-rh isn't "pretty" and adds extra markup, but a script tag is much more "markup" than extra data attributes.

So this option isn't one I would prefer, and probably wouldn't use. If the switch was flipped to make this default behavior, I think a why to "opt-out" of this behavior would be great, especially for people who always do SSR.

After being enlightened by @CanRau, (and reading the React Helmet issue linked above more carefully), I'm revising my opinion of option 2. This way ahead could actually be the best approach.

One possible solution to the overhead of including the tag's content would be to perform a very simple hash on the tag and/or attribute values. We use this algorithm for string hashing on both client and server (although we usually convert it to a string, like Math.abs(hash).toString(36)). It's super compact and fast.

// *server* would be a series of hashes like the below on each tag
hash('meta[name="blah-long-name"][content="lots of content here"]') => '[some short hash]'

// *client*
<script id="react-head-whitelist" type="text/template">
  [
    '[short hash]',
    '[short hash]',
    ...
  ]
</script>

On the client side, it would simply re-hash each tag checking for a match. If it matches, it's managed.

To make debugging easier, it would make sense to add an attribute or comment someplace indicating which tags are managed when NODE_ENV !== 'production'.

One other note... should the type attribute of that script tag be application/json instead of text/template?

Option 3 - Individually opt-in whitelist

I don't prefer this option since it requires "global" thinking at each component. If I'm writing a component that modifies the head tags, I don't really want to be thinking about how those head tags will be rendered.

In other words, I think the "whitelist" behavior should be abstracted away at the individual component level.

Option 4 - Nuclear!

I honestly like this approach:

  • Existing behavior is unaffected.
  • It removes the extra data-rh markup.
  • Doesn't require any additional page bloat (i.e. script tag).
  • Doesn't require any additional props at the component level.
  • It requires the least mental gymnastics. If you're using react-head to manage the head tags, and you want it to manage all the head tags, just flip the switch. It'll just work.

There's one case where I can imagine this wouldn't work well: If you're building some hybrid React application where React is only responsible for some portion of the page and not the entire page. For example, if I have a WordPress site and embed some React powered widget that uses react-head to manage head tags of the outer page. In this case, if you enable the nuclear option, it could potentially clobber the outer page's head tags. (But it's opt in... so... it's their own fault?)

Proposal: Option 5 - Nuclear + Whitelist

What if we had a hybrid Option 1 + Option 4 approach, with a manageAll and whitelist props? The manageAll prop would trigger the nuclear option, giving react-head permission to manage all head tags on the page. If the user needed more fine-grained control over what tags react-head is allowed to clobber, they could use the whitelist prop.

In this case, it should be an error to specify both the whitelist and manageAll props (mutually exclusive).

I like this approach because I think it provides a way to solve the problems presented and is incremental: existing behavior is preserved, users may allow react head to manage some or all of the head tags.

from react-head.

CanRau avatar CanRau commented on September 23, 2024

I've to add that that CSS like whitelist won't work like that at the moment and I don't know a simple solution to get it working as mentioned here #84 (comment) and possible workarounds here #84 (comment)

I'm actually open to any of those 😁I like the use case of a fallback a lot as well πŸ‘

Prop. 5: Nuke + whitelist sounds good but might be overkill to add both I don't know

The manageAll (I'd be fine with that name) could give more control by "just" adding everything using react-head you'd like it to manage, removing the possibility of duplicates

@JoshuaToenyes just a side note, maybe you already know(?!), it's not only about beauty but fear of SEO issues due to some software not properly scraping meta tags with additional attributes, that's at least what many are talking about in the above linked react-helmet issue

from react-head.

JoshuaToenyes avatar JoshuaToenyes commented on September 23, 2024

@CanRau Ah, I wasn't aware of the SEO concern or scraping problems on the data attributes. I will read through that issue more carefully!

Our software powers quite a few sites (~100) most using react-head and a few running react-helmet. I haven't noticed an SEO impact using either... but I'm also not doing any detailed SEO analysis on head tags either... I would be very interested if there's any data on that.

from react-head.

CanRau avatar CanRau commented on September 23, 2024

from react-head.

CanRau avatar CanRau commented on September 23, 2024

I know it's christmas just wanted to check for any updates on this?

from react-head.

tizmagik avatar tizmagik commented on September 23, 2024

Hey @CanRau thanks for following-up. Apologies but I haven't made progress on this, however, I think based on the discussion here we might at least have a path forward that I'd be open to a PR to add by someone, or I will try and get to myself if I get time.

Anyway, here's the key factors of what I think to be the best approach:

  • Easy mental model for users
  • Low overhead (both in terms of HTML payload and the developer experience)
  • Remove the need for data-rh attributes alltogether
  • Ability to manage all tags easily

I think a sort of super-hybrid of the Nuclear option and Auto-whitelist might work, sort of in line with the hybrid approach that @JoshuaToenyes suggested but combined to a single prop interface. Let me know what you think:

<HeadProvider headTags={headTags} manageTags={option}>

Where option is one of:

  • all: All tags are managed (we can make this the default in the next semver major)
  • types: Only the tags that are defined by react-head somewhere in the tree. For example, if you define a title tag via react-head, then all title tags are managed, while if you never specify any meta tags, then meta tags will remain untouched. This will avoid data-rh attributes.
  • specific: Individual and specific tags will be managed by react-head, this is equivalent to the current behavior. Requires using data-rh attributes.

What do you all think? Is this interface confusing? Does it cover all bases?

from react-head.

CanRau avatar CanRau commented on September 23, 2024

Think that looks good πŸ‘Not sure when I can find time to look into it though 😿
I'm working on a project again right now where I'm already using react-head with the Gatsby plugin, that might help get me working on this, no promises though πŸ˜…
At the moment I think I'd use types the most, as for the all I'd have to check if it'd mess with gatsby-plugin generated meta tags..

types may be called something like own or defined? Idk^^

from react-head.

osdiab avatar osdiab commented on September 23, 2024

Ran into this issue at my company - I'm glad to help but not sure what's the best avenue. For now planning to just wipe out the existing head tags on JS load to simulate the all managed option in our app, but not having the data attributes would be nice.

Thought for the "inferred"/"types" option (maybe infer can be the option name?) - is it planning to be at the granularity of "any meta tag" or something more specific like "meta tags with properties that were used"? If the latter, technically opengraph tags can be used as an "array" by having multiple of the same tag defined, which I don't personally use, but I guess wiping over existing stuff might do unexpected things for some users.

If a direction can be set then would be glad to try to contribute an implementation. Thanks!

from react-head.

tizmagik avatar tizmagik commented on September 23, 2024

Thanks for your interest, @osdiab ! Please feel free to submit a PR on this if you're up for it.

Thought for the "inferred"/"types" option (maybe infer can be the option name?)

I'm fine with infer as the option name, that works too. So it would be one of: all, infer or specific. Sounds good πŸ‘

is it planning to be at the granularity of "any meta tag" or something more specific like "meta tags with properties that were used"?

infer would opt react-head to manager all tags that were defined somewhere in the tree. So if your tree looked something like this:

<HeadProvider headTags={headTags} manageTags="infer">
  <App>
    // ... within any component, head tags are rendered as normal
    <Meta name="blah-long-name" content="lots of content here" />
    <Title>Title of page</Title>
  </App>
</HeadProvider>

And your HTML template looked something like this:

<head>
   <title>Fallback title</title>
   <link rel="stylesheet" type="text/css" href="theme.css">
</head>

Then the resultant tags on the page, after client rehydration would look like this:

<head>
   <meta name="blah-long-name" content="lots of content here" />
   <title>Title of page</title>
   <link rel="stylesheet" type="text/css" href="theme.css">
</head>

Note that meta and title tags, since they were defined in the JS are "opted-in" to be managed by react-head and any pre-existing tags in the HTML template (e.g. title) were removed. Meanwhile, since no <Link /> react-head tags were defined, the link tags were unaffected.


Let me know if that's still unclear. Thanks!

from react-head.

duythinh-dev avatar duythinh-dev commented on September 23, 2024

I need to help :(
Im making website https://www.hahalolo.com/tour
I setting all meta tags but when i check in developers.facebook.com/tools/debug
i check on fb it returns me results of website hahalolo.com

from react-head.

Related Issues (20)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    πŸ–– Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. πŸ“ŠπŸ“ˆπŸŽ‰

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❀️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.