fix: add whitelist prop to HeadProvider

[CanRau]

<HeadProvider/> accepts additional prop whitelist which takes a CSS selector string (used in document.querySelector) to disable data-rh for identification on whitelisted tags

As mentioned here #40
based on nfl/react-helmet#79

As it doesn't seem too heavy, I think it's a good addition which could make many former react-helmet users pretty happy

I plan another PR soon'ish🤞 to add support for html & body attributes as mentioned here #40
I'm working on a new website for our org right now and am evaluating plugins one by one, not just copying everything from the main website.

If all this gets merged I would publish a gatsby-plugin-react-head, I've already prepared, which could make react-head more interesting to many gatsbys 💜

Edit: Forget to mention that I'm of course open to feedback about everything.
For example I'm not sure about whitelist being the best name, so any suggestions are highly appreciated 🙏 and code style changes are welcome as well, it's kind of a POC..

[tizmagik]

Hey this seems really neat @CanRau -- thanks for submitting the PR! Also excited to hear about a potential gatsby-plugin-react-head, that's pretty cool!

Question about the whitelist API and then I think this will need some additional tests to cover the changes, otherwise this LGTM and I'd be happy to add this additional capability.

[CanRau]

Kay takes array instead of string now 👍
Did some "cleanup" as well I think it's a little easier to read now..especially that part in HeadTag.js
And updated the readme.

Got a side question, still haven't been able to publish the website including the gatsby-plugin yet, so nothing to link to for now.
Does it matter if we pass an empty headTags array on the client side as well?
I mean would it hurt to add headTags={[]} to client.js?
Doesn't seem to make a difference so far, just wanted to make sure as I use the same component to wrap client and server in the plugin..

Edit: Uh, I forgot the tests, looking into them now

[CanRau]

Kay added some tests, I'm still not as much into testing as I would love to be, so let me know if they're okay and sufficient like this or what else I could improve 🙏

[CanRau]

Like so https://github.com/tizmagik/react-head/pull/84/files#diff-04c6e90faac2675aa89e2176d2eec7d8R88 ?

Should I merge in the latest commit, with the headTags clarification?

[CanRau]

@tizmagik Any update on a possible merge? Should I merge the last changes to master beforehand?

[tizmagik]

Hey sorry @CanRau , I haven't forgotten about this!

Curious if you think getting started on that gatsby plugin before this is released against latest would make sense? I'm thinking it might inform if this API is sufficient or if there's a better approach once there's a concrete use case? If not, also happy to move forward with this as-is

[CanRau]

The plugin is basically done https://github.com/GaiAma/Coding4GaiAma/tree/master/packages/gatsby-plugin-react-head
just not yet published, as I thought to have this and attributes functionality, I haven't worked so far, merged before
It's already in use here https://coding4.gaiama.org, it's a new page I'm working on..haven't told anyone so far and isn't even using many meta tags so far so probably not what you were thinking^^

Yeah I guess I can get the plugin published, only thing I'm not sure, how are people going to use this PR then before it's merged? I've got a local fork in the same repo to make it work
Any thoughts? Release under another tag?

Or did I misunderstand your request? You're interested in the plugin being used more to know if that API makes sense like this right?

[tizmagik]

Yes I can release under a next tag that you can install via npm i react-head@next — I’ll ping you when that’s up

[CanRau]

Awesome than I update the plugin to depend on @next and publish it 🎉

[tizmagik]

@CanRau ok this is available now as npm i react-head@next -- please give that a spin and if it looks good we can tag a full release. Sorry again for the slow responses on this!

[CanRau]

Awesome thank you 🙏 Don't worry all good 👍

I'm testing it locally right now...for whatever reason my plugin broke..I'll have breakfast now and then figure out how to fix it..

By the way I hope I didn't mess up those merges 😕

[tizmagik]

I released from your branch at this commit, 8ba9786 I can do another release at the latest commit if you'd like. Let me know

[tizmagik]

Ok just published another preminor (I guess it should have been prepatch), so
8ba9786 is available as 3.2.0-0 and a3aba7b as 3.3.0-0

[CanRau]

I'm embarrassed to say I have no idea what's wrong 😭
Version 3.1.1 as well as my local version work fine with my gatsby-plugin, the latter is what's currently running on coding4.gaiama.org.

I've got a separate directory on my machine with the fork I used to update the tests etc and push changes to this PR.

When yarn add react-head@next or trying to yarn link my fork then yarn link react-head in my local coding4.gaiama.org it crashes

<HeadProvider /> should be in the tree

Even more confusing, react-dev-tools shows me the HeadProvider component and I can see the Context.Provider with all the props/state of react-head

When I manually wrap <HeadProvider/> around my app it works with all versions (if I remember correctly) but that's what the gatsby-plugin is supposed to do and does correctly using the local version and 3.1.1, not changing anything in the gatsby-plugin..

I'm pretty confused and don't know what I'm doing wrong
Not sure you can help, just so you know..
As we're on a short trip from tomorrow until ~Wednesday further investigation has to wait unfortunately..
Thought all this would just be a matter of some commands, pushes and a publish 🤦‍♂haha

[tizmagik]

Hey no worries, I appreciate your efforts here and I'm sure we'll figure out what's going on, there's no rush 😁

If I find some time I'll try and dig in and try to repro. Where is the gatsby plugin/what repro steps could I look at?

[CanRau]

Wow, that was a long time, didn't expect that, I know you said "no worries" yet I'm sorry for just "disappearing" 🤦‍♂ a lot going on here in the jungle right now, then my mom is visiting us and they're cutting electricity for about 15 days from next Saturday it seems 😦

So if you find time and curiosity to try to repro it I'd say this would be the easiest GaiAma/Coding4GaiAma cause it should contain everything

I'll return as quick as possible which might take another couple of weeks unfortunately though
So if you figure it out and feel like merging it go for it..I should be able to comment (and help with the repo if needed) if no coding is required as I'm mostly on my phone for travel reasons..

Thanks a lot for your patience 🙏
I have to say the coder world seems so much more friendly and relaxed compared to other communities 😅 I know it's not always the case though

See you soon'ish 💯

[tizmagik]

Not a problem at all. There’s no rush on this so take your time.

My wife and I actually just had our baby girl so I will be out on parental leave for several weeks anyway 😊

[CanRau]

Congratulations 🎉 enjoy yourselves then ❤️

[CanRau]

Long time no see 😱 How are you and your family doing? 😀

I have to admit I'm not sure what was going on when I last worked on this feature 🤷‍♂️
Anyway so I published my gatsby plugin to npm and github to see what happens..after some back and forth I got it kind of working just to find out that the whole approach isn't working at all or is at least heavily flawed^^
That headTags.whitelist.includes(Tag) in https://github.com/tizmagik/react-head/pull/84/files#diff-f85f4e10a789184c818ff777c38622efR43 is of course not working, at least not using my envisioned whitelist like this

const whitelist = [
  `title`,
  `[name="description"]`,
  `[property^="og:"]`
]

As Tag can only match title and the rest is an object like

{
  name: 'description',
  content: 'My fancy description',
}

Where tag would be meta in this case.
I'm not exactly sure what's the best approach to match those properly against that whitelist.

Do you have any thoughts on this or ideas?

Cause I think turning that whitelist into just tags would be too loose and we're using it in querySelectorAll

Thought I could come up with a clever .reduce way of turning those rest objects into something but those selectors look intimidating 😅😭

[CanRau]

Maybe whitelist could look like

const whitelist = {
  title: true,
  meta: [
    { attr: 'name', value: 'description' },
    { attr: 'property', value: 'og:*' },
  ],
}

basic globbing/regex support? Or just do a ''.startsWith() to compare
or even

const whitelist = {
  title: true,
  meta: {
    name: 'description',
    property: 'og:*',
  },
}

And we could provide a default whitelist people could just import

[tizmagik]

Hmm... admittedly I'm context switching a bunch and haven't been working too closely to this code, but I wonder if we could somehow support this with an elegant API rather than an explicit whitelist (would that be more desirable?) What do you think about having it as a prop on each react-head element so you can opt-in/out as you go?

import { Meta } from 'react-head';

 <Meta name="example" content="whatever" whitelist />

Maybe whitelist doesn't make sense in such an API, perhaps raw or plain or some other way to denote it?

I think this approach may require additional context to be passed in on the client render, so not sure if that's more or less desirable. Thoughts?

[CanRau]

Interesting, yea I think I like that a lot better as it makes it more explicit.
Only the term I'm not sure about.

Wait, I'm a little confused now ^^ That API would then kind of auto generate the whitelist correct?
If that works easily (with SSR) how about just using this technique by default without any API?

[tizmagik]

Wait, I'm a little confused now ^^ That API would then kind of auto generate the whitelist correct?
If that works easily (with SSR) how about just using this technique by default without any API?

The issue is, as to how react-head works currently, it only manages tags that it knows that it created (e.g. tags with data-rh attribute). By default, it does not touch tags that it hasn't placed on the page itself (e.g. if there's tags that are part of the static html template).

I think I have an idea of how to manage this. Let me write up a separate issue (and I'll post back here) with more details, and we can discuss/vote on options there. Give me a few minutes...

[tizmagik]

Wrote up a quick RFC: #97