react-helmet-async icon indicating copy to clipboard operation
react-helmet-async copied to clipboard

getTagsFromPropsList behavior depends on Object.keys order of tags

Open jordanrastrick opened this issue 6 years ago • 0 comments

Because the primaryAttributeKey is determined by side effects while looping over Object.keys(tag), it matters what order those keys are generated in, which I don't think is going to be the expected behavior. For example, in this unit test, if on line 366 href: null is moved to the front of the object, when I run the test it no longer passes.

Unfortunately I am not quite familiar enough yet with this library or the specifics of how all the <head> tags work to make the decisions needed for a PR that would address this problem (e.g. in the case of link since rel and href are both "primary" should both be stored during the loop, one as the deterministically chosen primary and the other just to check for null values, per that unit test? Not sure....)

I can see this code was inherited from react-helmet but am not sure if there's any benefit trying to engage the original authors there given it doesn't look to be actively maintained anymore.

For context, I came across this issue as an obstacle when trying to work on a Pull Request that introduces a new custom attribute which when attached to script tags lets the most deeply nested one overwrite others with the same attribute, similar to the existing behavior for other tags. This is motivated by wanting to work with the same semantics in <Helmet> that say the <meta> tag gives for Opengraph properties, in the context of the <script> tag for Google's structured data. I would be happy to submit PRs both to fix the blocking issue and also to introduce this feature, if some decision can be made about what approach to take with the former, and if the latter would be a welcome addition.

jordanrastrick avatar Nov 10 '19 10:11 jordanrastrick