vercel / next.js

The React Framework
https://nextjs.org
MIT License
121.8k stars 26.05k forks source link

Allow next/head to render react components #17770

Closed jflayhart closed 2 years ago

jflayhart commented 3 years ago

ℹ️ There is an easy "workaround" you can apply instead of using React components, which are NOT supported within next/head as of NextJS 10.2

Fixes: #17721 Fixes: #17842

Essentially, >=9.5.4 breaks compatibility for having react components within Head. While it's true that NextJS never truly supported this...

title, meta or any other elements (e.g. script) need to be contained as direct children of the Head element, or wrapped into maximum one level of or arrays—otherwise the tags won't be correctly picked up on client-side navigations.

...this is a fundamental React principle and we should support the concept of component composition. For example,

const HomePage = () => (
    <>
        <Head>
            <CustomTitle />
            <ThirdPartyScripts />
        </Head>
        <div>Homepage</div>
    </>
)

I tracked the issue to https://github.com/vercel/next.js/pull/16758/files#diff-f0591f99e1e66f02b12d5f64977b579eR86 where apparently we serialize head data in the window object now: window.__NEXT_DATA__, which doesn't handle components. I'm not sure the best way to handle that, but I created logic in head-manager.ts to handle react components and their potential children.

TODO

ijjk commented 3 years ago

Stats from current PR

Default Server Mode (Increase detected ⚠️)
General Overall increase ⚠️ | | vercel/next.js canary | jflayhart/next.js fix-head-component-bug | Change | | - | - | - | - | | buildDuration | 11.8s | 11.6s | -238ms | | nodeModulesSize | 63.4 MB | 63.4 MB | ⚠️ +296 B |
Page Load Tests Overall decrease ⚠️ | | vercel/next.js canary | jflayhart/next.js fix-head-component-bug | Change | | - | - | - | - | | / failed reqs | 0 | 0 | ✓ | | / total time (seconds) | 2.198 | 2.234 | ⚠️ +0.04 | | / avg req/sec | 1137.36 | 1118.93 | ⚠️ -18.43 | | /error-in-render failed reqs | 0 | 0 | ✓ | | /error-in-render total time (seconds) | 1.184 | 1.193 | ⚠️ +0.01 | | /error-in-render avg req/sec | 2111.14 | 2096.01 | ⚠️ -15.13 |
Client Bundles (main, webpack, commons) Overall increase ⚠️ | | vercel/next.js canary | jflayhart/next.js fix-head-component-bug | Change | | - | - | - | - | | 677f882d2ed8..9b19.js gzip | 11.1 kB | 11.1 kB | ✓ | | framework.HASH.js gzip | 39 kB | 39 kB | ✓ | | main-51c1c2f..7e91.js gzip | 7.23 kB | 7.24 kB | ⚠️ +14 B | | webpack-e067..f178.js gzip | 751 B | 751 B | ✓ | | Overall change | 58 kB | 58 kB | ⚠️ +14 B |
Client Bundles (main, webpack, commons) Modern Overall increase ⚠️ | | vercel/next.js canary | jflayhart/next.js fix-head-component-bug | Change | | - | - | - | - | | 677f882d2ed8..dule.js gzip | 6.9 kB | 6.9 kB | ✓ | | framework.HA..dule.js gzip | 39 kB | 39 kB | ✓ | | main-87d379f..dule.js gzip | 6.29 kB | 6.29 kB | ⚠️ +8 B | | webpack-07c5..dule.js gzip | 751 B | 751 B | ✓ | | Overall change | 52.9 kB | 52.9 kB | ⚠️ +8 B |
Legacy Client Bundles (polyfills) | | vercel/next.js canary | jflayhart/next.js fix-head-component-bug | Change | | - | - | - | - | | polyfills-4b..e242.js gzip | 31 kB | 31 kB | ✓ | | Overall change | 31 kB | 31 kB | ✓ |
Client Pages | | vercel/next.js canary | jflayhart/next.js fix-head-component-bug | Change | | - | - | - | - | | _app-9a0b9e1..b37e.js gzip | 1.28 kB | 1.28 kB | ✓ | | _error-ed1b0..8fbd.js gzip | 3.44 kB | 3.44 kB | ✓ | | hooks-89731c..c609.js gzip | 887 B | 887 B | ✓ | | index-17468f..5d83.js gzip | 227 B | 227 B | ✓ | | link-409b283..e3ab.js gzip | 1.32 kB | 1.32 kB | ✓ | | routerDirect..924c.js gzip | 284 B | 284 B | ✓ | | withRouter-7..c13d.js gzip | 284 B | 284 B | ✓ | | Overall change | 7.73 kB | 7.73 kB | ✓ |
Client Pages Modern | | vercel/next.js canary | jflayhart/next.js fix-head-component-bug | Change | | - | - | - | - | | _app-75d3a82..dule.js gzip | 625 B | 625 B | ✓ | | _error-4469a..dule.js gzip | 2.29 kB | 2.29 kB | ✓ | | hooks-cbf13f..dule.js gzip | 387 B | 387 B | ✓ | | index-b9a643..dule.js gzip | 226 B | 226 B | ✓ | | link-92d3016..dule.js gzip | 1.28 kB | 1.28 kB | ✓ | | routerDirect..dule.js gzip | 284 B | 284 B | ✓ | | withRouter-f..dule.js gzip | 282 B | 282 B | ✓ | | Overall change | 5.37 kB | 5.37 kB | ✓ |
Client Build Manifests | | vercel/next.js canary | jflayhart/next.js fix-head-component-bug | Change | | - | - | - | - | | _buildManifest.js gzip | 323 B | 323 B | ✓ | | _buildManife..dule.js gzip | 329 B | 329 B | ✓ | | Overall change | 652 B | 652 B | ✓ |
Rendered Page Sizes Overall increase ⚠️ | | vercel/next.js canary | jflayhart/next.js fix-head-component-bug | Change | | - | - | - | - | | index.html gzip | 1 kB | 1 kB | ⚠️ +1 B | | link.html gzip | 1.01 kB | 1.01 kB | ✓ | | withRouter.html gzip | 997 B | 997 B | ✓ | | Overall change | 3.01 kB | 3.01 kB | ⚠️ +1 B |
#### Diffs
Diff for main-8e73dd2..68413e28f.js ```diff @@ -150,6 +150,10 @@ _N_E = (window["webpackJsonp_N_E"] = window["webpackJsonp_N_E"] || []).push([ return; } + if (typeof tag.type === "function") { + return; + } + var newTag = reactElementToDOM(tag); var elementIter = elements.values(); @@ -186,13 +190,20 @@ _N_E = (window["webpackJsonp_N_E"] = window["webpackJsonp_N_E"] || []).push([ var elements = new Set(headEl.children); updateElements( elements, - initialHeadEntries.map(function(_ref2) { - var _ref3 = _slicedToArray(_ref2, 2), - type = _ref3[0], - props = _ref3[1]; + initialHeadEntries + .filter(function(_ref2) { + var _ref3 = _slicedToArray(_ref2, 1), + type = _ref3[0]; - return /*#__PURE__*/ (0, _react.createElement)(type, props); - }), + return type; + }) + .map(function(_ref4) { + var _ref5 = _slicedToArray(_ref4, 2), + type = _ref5[0], + props = _ref5[1]; + + return /*#__PURE__*/ (0, _react.createElement)(type, props); + }), false ); var updatePromise = null; ```
Diff for main-f21a93d..8d.module.js ```diff @@ -121,6 +121,10 @@ return; } + if (typeof tag.type === "function") { + return; + } + var newTag = reactElementToDOM(tag); var elementIter = elements.values(); @@ -155,10 +159,15 @@ var elements = new Set(headEl.children); updateElements( elements, - initialHeadEntries.map(_ref2 => { - var [type, props] = _ref2; - return /*#__PURE__*/ (0, _react.createElement)(type, props); - }), + initialHeadEntries + .filter(_ref2 => { + var [type] = _ref2; + return type; + }) + .map(_ref3 => { + var [type, props] = _ref3; + return /*#__PURE__*/ (0, _react.createElement)(type, props); + }), false ); var updatePromise = null; ```
Diff for index.html ```diff @@ -6,7 +6,7 @@ @@ -85,13 +85,13 @@ src="/_next/static/chunks/polyfills-f73ba3fc145972ef83e9.js" > Githubissues.
  • Githubissues is a development platform for aggregating issues.