Skip to content
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

Helmet 5.0.0: The no-API release #246

Merged
merged 52 commits into from
Mar 21, 2017
Merged

Helmet 5.0.0: The no-API release #246

merged 52 commits into from
Mar 21, 2017

Conversation

doctyper
Copy link
Contributor

@doctyper doctyper commented Mar 16, 2017

Helmet 5

@cwelch5 and I got inspired during React Conf 2017 and worked together to overhaul the Helmet API. This release will replace the current API with, well, no API (...sort of).

Helmet 5 will allow you to rewrite this:

<Helmet
    title="My Title"
    meta={[
        {name: "description", content: "Helmet application"}
    ]}
/>

to this:

<Helmet>
    <title>My Title</title>
    <meta name="description" content="Helmet application" />
</Helmet>

That's it. There's no need to pass in props or large objects / arrays. Helmet now takes plain HTML tags and outputs plain HTML tags. It's dead simple, and React beginner friendly.

Minimal API

The Helmet API has been reduced to just two optional convenience props and an optional callback:

<Helmet
    defaultTitle="My Default Title"
    titleTemplate="MySite.com - %s"
    onChangeClientState={(newState) => console.log(newState)}
/>

Reference Example

For a more concrete example, here is the current README example converted to Helmet 5:

<Helmet
    titleTemplate="MySite.com - %s"
    defaultTitle="My Default Title"
    onChangeClientState={(newState) => console.log(newState)}
>
    <html lang="en" amp />

    <title itemProp="name" lang="en">My Title</title>

    <base target="_blank" href="https://mysite.com/" />

    <meta name="description" content="Helmet application" />
    <meta property="og:type" content="article" />

    <link rel="canonical" href="https://mysite.com/example" />
    <link rel="apple-touch-icon" href="https://mysite.com/img/apple-touch-icon-57x57.png" />
    <link rel="apple-touch-icon" sizes-"72x72" href="https://mysite.com/img/apple-touch-icon-72x72.png" />

    <script src="https://include.com/pathtojs.js" type="text/javascript" />
    <script type="application/ld+json">{`
        { "@context": "https://schema.org" }
    `}</script>

    <noscript>{`
        <link rel="stylesheet" type="text/css" href="foo.css" />
    `}</noscript>

    <style type="text/css">{`
        "body {background-color: blue;} p {font-size: 12px;}"
    `}</style>
</Helmet>

Todo

We have a few more items to check off, but we are excited for this release!

  • Rewrite Helmet API
    • Maintain backward compatibility with Helmet 4
    • Ensure unit tests pass using either API
    • Document the new API
  • Add Helmet.renderStatic() alias to Helmet.rewind() to reduce confusion on what it does
  • Add a {Helmet} export for full ES Module compatibility
  • Add support for <body> attributes
  • Use requestIdleCallback for DOM writes

Preview

Please feel free to help us out by installing and test-driving our preview release. Feedback is much appreciated!

yarn add react-helmet@next

or:

npm install react-helmet@next

cwelch5 and others added 30 commits March 14, 2017 12:46
…ating a more declarative API. Fully backwards compatible.
…' of github.com:nfl/react-helmet into feature/declarative-api
@CLAassistant
Copy link

CLAassistant commented Mar 16, 2017

CLA assistant check
All committers have signed the CLA.

Copy link
Contributor

@cwelch5 cwelch5 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nicely done. Thanks for formalizing our hack session. Looks great!

Just a couple typos and you should be good to go.

src/Helmet.js Outdated
* @param {Array} noscript: [{"innerHTML": "<img src='https://mysite.com/js/test.js'"}]
* @param {Array} style: [{"type": "text/css", "cssText": "div{ display: block; color: blue; }"}]
* @param {Array} meta: [{"name": "description", "content": "Test description"}]
* @param {Array} noscript: [{TAG_PROPERTIES.INNER_HTML: "<img src='https://mysite.com/js/test.js'"}]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"innerHTML"

src/Helmet.js Outdated
* @param {Function} onChangeClientState: "(newState) => console.log(newState)"
* @param {Array} script: [{"type": "text/javascript", "src": "https://mysite.com/js/test.js"}]
* @param {Array} style: [{"type": "text/css", TAG_PROPERTIES.CSS_TEXT: "div{ display: block; color: blue; }"}]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"cssText"

HelmetExport.renderStatic = HelmetExport.rewind;

export {HelmetExport as Helmet};
export default HelmetExport;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

well played

@doctyper doctyper merged commit 22f6806 into master Mar 21, 2017
@doctyper doctyper mentioned this pull request Mar 21, 2017
@nlubin-lv
Copy link

This seems to have been committed to master causing some confusion with the readme (instead of to a branch for the new release)

One such confusion: #249

@doctyper
Copy link
Contributor Author

The React community is too quick, I literally landed this a few hours ago! We'll be publishing the new API shortly.

@nlubin-lv
Copy link

nlubin-lv commented Mar 21, 2017

@doctyper consider a next branch next time :-) -- or release branches.

I spent an hour this morning trying to figure out why my code wasnt working only to realize that the versions did not match up :-(

@doctyper
Copy link
Contributor Author

I do apologize, but do note that master branches do not always align with currently published versions. This goes for any repository. I would instead direct you to the npm-hosted README, which does currently point to the published release. https://www.npmjs.com/package/react-helmet

As a general policy we do not use release branches @nfl.

@nlubin-lv
Copy link

I understand internally @nfl may not use release branches, but as a consumer of your open source code my team cannot use a tool that introduces breaking changes into master with no warning

@nlubin-lv
Copy link

@doctyper We do like the project though and we use it extensively -- not saying we are jumping ship :-)

Just raising concerns is all as breaking changes on master can get the community a little confused without a release.

@cwelch5
Copy link
Contributor

cwelch5 commented Mar 21, 2017

@nlubin-lv Thanks for keeping us honest, we should have held off on the README changes until we bumped the version.

@ntucker
Copy link

ntucker commented Mar 30, 2017

Hmm, this is rather confusing because I thought I could then put some React components in there, but I'm getting Helmet expects a string as a child of <title>. Did you forget to wrap your children in braces?

So using react-intl FormattedMessage doesn't work which would be what I thought the point of this would be

@nlubin-lv
Copy link

nlubin-lv commented Mar 30, 2017

@ntucker it seems you can only put the components that react-helmet accepts as children to Helmet and not any element.

I believe that was on purpose as not any element can go in the head tag.

I needs a string as a child of title due to the fact that the title gets manipulated by the module to add on attributes and other data.

@kristiehoward
Copy link

kristiehoward commented Jul 27, 2017

In case it is helpful to anyone in the future, I wrote a codemod to transform all of our react-helmet components to the new version 5 API. We had ~40 files using react-helmet and it made the transition to the v5 API ridiculously easy.
https://github.com/kristiehoward/react-helmet-v5-codemod

@tmbtech tmbtech deleted the feature/declarative-api branch May 1, 2019 00:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants