3.0 api about react-head

[TrySound]

forEach on NodeList too. Let's keep as is until somebody will request ie support.

[tizmagik]

Awesome, great work @TrySound -- will cut a release as soon as I get a moment

[tizmagik]

Should we provide default prop for headTags with empty array?

This makes sense, however it should warn/throw an error on the server if the prop is not provided in those cases since this means misconfigured server rendering. Maybe can do a typeof window check and default to empty array on client and throw when on the server?

Should we enforce having HeadProvider in the tree by throwing an error if context value is null?

The only valid use case for not having HeadProvider in the tree is for cases when a user doesn't care about the cascade, right? Which we can treat as a misconfiguration and then throwing an error would make sense. Any other alternatives? Would a console.error not be enough you think?

Should we throw and error in componentDidUpdate if tag prop is changed or just remove HeadTag from public api and provide more aliases?

What are the cases where the tag prop would change? Are there any valid uses cases for this? Since we already provide all of the W3C-compliant head attributes it may make sense to either remove the HeadTag from the public api or at least not publicize it in the documentation as much.

[TrySound]

@tizmagik Do we care about ie11 support? We use .remove() method which is not supported there.

[tizmagik]

Hmm yea good point, I personally don’t care about IE11 support, but if that’s the only thing that’s breaking support maybe we could address it? What do you think @TrySound ?

[TrySound]

@tizmagik I think we are ready for release.

[tizmagik]

Just a quick update here, a couple more things I want to get done before a 3.0 release

• Update example app to support new API
• Some sort of migration guide (maybe can be included with the release notes)

Will try and get to these ASAP, unless someone else wants to (feel free).

[tizmagik]

I was working through upgrading the example app and it struck me that <HeadProvider /> really feels like an implementation detail.

What do you think about renaming it to <ReactHead /> and making it the default export (as part of 3.0 release), e.g.

/* client.js */
import React from 'react';
import ReactHead, { Title, Link, Meta } from 'react-head';

const App = () => (
  <ReactHead>
    <div className="Home">
      <Title>Title of page</Title>
      <Link rel="canonical" content="http://jeremygayed.com/" />
      <Meta name="example" content="whatever" />
      // ...
    </div>
  </ReactHead>
);

[TrySound]

Nope.

1. default exports makes distribution trickier. cjs output will have default export which should be handled by tools. And for example webpack is not able to bundle well cjs which imports esm with default export. In any case I also think default exports is a wrong part of the spec and makes module system too complex.
2. Provider name is like a pattern. It's used in redux for years and used by react itself in context api. It's better to keep descriptive name HeadProvider.
3. React in components names is overnaming. All exported components are react.

[TrySound]

The necessity of HeadProvider is defined by invariant. It should not be a problem for user.

[tizmagik]

Ok that makes sense, you convinced me to keep the name as-is 😁

I'm having a bit of trouble upgrading the example app but should hopefully get it going soon. I can cut another pre-release if you're itching to use the latest though. Lmk

[tizmagik]

Ok example app updated. I think there's issues with react-router and the new React Context API or something, so I opted for @reach/router; #60

[TrySound]

Yes, this is known problem. Old context providers cannot be inside the new api provider.

[TrySound]

Hm, should be fixed in 16.4.
facebook/react#12551 (comment)

[tizmagik]

v3.0.0 has been released 🎉