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

Add default utf-8 charset. #270

Merged
merged 1 commit into from
Nov 17, 2016
Merged

Conversation

zpnk
Copy link
Contributor

@zpnk zpnk commented Nov 17, 2016

Closes #237.

Sets UTF-8 as the default charset for pages without a <Head> or with a <Head> but no <meta charSet>.

If <Head> has a <meta charSet> tag, that value will override the default.

Thanks for next! 😄

@@ -33,7 +33,7 @@ export async function render (url, ctx = {}, {
return (staticMarkup ? renderToStaticMarkup : renderToString)(app)
})

const head = Head.rewind() || []
const head = Head.rewind() || defaultHead()
Copy link
Contributor Author

@zpnk zpnk Nov 17, 2016

Choose a reason for hiding this comment

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

We still want to use the default head tag(s) even if the page does not have a <Head>.

@rauchg
Copy link
Member

rauchg commented Nov 17, 2016

Unfortunately it has to be a little bit more subtle: we set a default charset unless the user explicitly passes a <meta> with charSet.

The reason for this is that <Head> is a HOC that appends, does not set (i.e.: it introduces side effects)

So, instead of defaultHead, your code should have a Boolean like .userSetCharSet perhaps.

@rauchg
Copy link
Member

rauchg commented Nov 17, 2016

Also: in this day and age, having a default of utf-8 should not come as a surprise to anyone. That said, it might be prudent to do something like:

console.info('Using default `<meta charSet="utf-8">`. Learn more: https://github.com/zeit/next.js/wiki/utf-8-charset')

@rauchg
Copy link
Member

rauchg commented Nov 17, 2016

during dev, of course

@zpnk
Copy link
Contributor Author

zpnk commented Nov 17, 2016

@rauchg Thanks for the feedback!

Unfortunately it has to be a little bit more subtle: we set a default charset unless the user explicitly passes a <meta> with charSet.

I believe I'm handling this requirement already. The default is appended to the children of <Head> here. The resulting array is then filtered for uniqueness. Since the default comes last, it will only be kept if a <meta> tag with the charSet property is not given in the user's <Head>.

The defaultHead method is introduced only to supply the charset default for pages without a <Head> component in them. (Could also be used to add other sensible defaults for <Head>-less pages)

The scenarios I've tried to cover are:

1.) A simple page component such as export default () => <h1>My component!</h1> will receive the charset default. (via defaultHead)
2.) A page component with <Head> set, but no charset will receive the default charset:

<Head>
  <title>My title</title>
</Head>
  1. A page component with <Head> set and a charset will respect that charset:
<Head>
  <title>My title</title>
  <meta charSet="utf-2016" />
</Head>

I attempted to express that with the tests I added, but perhaps I could improve them. (Suggestions welcome)

If this approach does not meet exceptions, could you please elaborate on the userSetCharSet concept? I'm having trouble visualizing how that would work. Thanks! 😄

@rauchg
Copy link
Member

rauchg commented Nov 17, 2016

Sorry. File my comment under "read first 5 lines of the diff and jumped to conclusions". Amazing PR. Will merge from again. A++++++++!+!==

@rauchg rauchg merged commit e045582 into vercel:master Nov 17, 2016
@jimthedev
Copy link

Thanks!

@lock lock bot locked as resolved and limited conversation to collaborators Jan 19, 2019
@ijjk
Copy link
Member

ijjk commented Oct 4, 2019

Stats from current PR

Default Server Mode
General
zeit/next.js canary zeit/next.js canary Change
buildDuration 15.6s 15.9s ⚠️ +335ms
nodeModulesSize 48.3 MB 48.3 MB
Client Bundles (main, webpack, commons)
zeit/next.js canary zeit/next.js canary Change
main-HASH.js 18.9 kB 18.9 kB
main-HASH.js gzip 6.79 kB 6.79 kB
webpack-HASH.js 1.53 kB 1.53 kB
webpack-HASH.js gzip 746 B 746 B
4952ddcd88e7..2b8407376.js 21.9 kB 21.9 kB
4952ddcd88e7..7376.js gzip 7.81 kB 7.81 kB
de003c3a9d30..6604acae7.js 43.2 kB 43.2 kB
de003c3a9d30..cae7.js gzip 15.5 kB 15.5 kB
framework.5b..dbaff70d3.js 125 kB 125 kB
framework.5b..70d3.js gzip 39.4 kB 39.4 kB
Overall change 211 kB 211 kB
Client Bundles (main, webpack, commons) Modern
zeit/next.js canary zeit/next.js canary Change
main-HASH.module.js 17.2 kB 17.2 kB
main-HASH.module.js gzip 6.52 kB 6.52 kB
webpack-HASH.module.js 1.53 kB 1.53 kB
webpack-HASH..dule.js gzip 746 B 746 B
de003c3a9d30..42.module.js 45.6 kB 45.6 kB
de003c3a9d30..dule.js gzip 16.5 kB 16.5 kB
framework.5b..d3.module.js 125 kB 125 kB
framework.5b..dule.js gzip 39.4 kB 39.4 kB
Overall change 190 kB 190 kB
Client Pages
zeit/next.js canary zeit/next.js canary Change
_app.js 1.81 kB 1.81 kB
_app.js gzip 873 B 873 B
_error.js 12 kB 12 kB
_error.js gzip 4.73 kB 4.73 kB
hooks.js 12.7 kB 12.7 kB
hooks.js gzip 4.79 kB 4.79 kB
index.js 318 B 318 B
index.js gzip 222 B 222 B
link.js 8.14 kB 8.14 kB
link.js gzip 3.5 kB 3.5 kB
routerDirect.js 408 B 408 B
routerDirect.js gzip 281 B 281 B
withRouter.js 419 B 419 B
withRouter.js gzip 280 B 280 B
Overall change 35.8 kB 35.8 kB
Client Pages Modern
zeit/next.js canary zeit/next.js canary Change
_app.module.js 1.7 kB 1.7 kB
_app.module.js gzip 832 B 832 B
_error.module.js 23.3 kB 23.3 kB
_error.module.js gzip 8.59 kB 8.59 kB
hooks.module.js 1.52 kB 1.52 kB
hooks.module.js gzip 793 B 793 B
index.module.js 294 B 294 B
index.module.js gzip 223 B 223 B
link.module.js 8.53 kB 8.53 kB
link.module.js gzip 3.68 kB 3.68 kB
routerDirect.module.js 394 B 394 B
routerDirect..dule.js gzip 281 B 281 B
withRouter.module.js 404 B 404 B
withRouter.m..dule.js gzip 278 B 278 B
Overall change 36.1 kB 36.1 kB
Client Build Manifests
zeit/next.js canary zeit/next.js canary Change
_buildManifest.js 81 B 81 B
_buildManifest.js gzip 61 B 61 B
_buildManifest.module.js 81 B 81 B
_buildManife..dule.js gzip 61 B 61 B
Overall change 162 B 162 B
Rendered Page Sizes
zeit/next.js canary zeit/next.js canary Change
index.html 3.62 kB 3.62 kB
index.html gzip 945 B 945 B
link.html 3.66 kB 3.66 kB
link.html gzip 954 B 954 B
withRouter.html 3.67 kB 3.67 kB
withRouter.html gzip 942 B 942 B
Overall change 10.9 kB 10.9 kB

Serverless Mode
General
zeit/next.js canary zeit/next.js canary Change
buildDuration 15.7s 16s ⚠️ +290ms
nodeModulesSize 48.3 MB 48.3 MB
Client Bundles (main, webpack, commons)
zeit/next.js canary zeit/next.js canary Change
main-HASH.js 18.9 kB 18.9 kB
main-HASH.js gzip 6.79 kB 6.79 kB
webpack-HASH.js 1.53 kB 1.53 kB
webpack-HASH.js gzip 746 B 746 B
4952ddcd88e7..2b8407376.js 21.9 kB 21.9 kB
4952ddcd88e7..7376.js gzip 7.81 kB 7.81 kB
de003c3a9d30..6604acae7.js 43.2 kB 43.2 kB
de003c3a9d30..cae7.js gzip 15.5 kB 15.5 kB
framework.5b..dbaff70d3.js 125 kB 125 kB
framework.5b..70d3.js gzip 39.4 kB 39.4 kB
Overall change 211 kB 211 kB
Client Bundles (main, webpack, commons) Modern
zeit/next.js canary zeit/next.js canary Change
main-HASH.module.js 17.2 kB 17.2 kB
main-HASH.module.js gzip 6.52 kB 6.52 kB
webpack-HASH.module.js 1.53 kB 1.53 kB
webpack-HASH..dule.js gzip 746 B 746 B
de003c3a9d30..42.module.js 45.6 kB 45.6 kB
de003c3a9d30..dule.js gzip 16.5 kB 16.5 kB
framework.5b..d3.module.js 125 kB 125 kB
framework.5b..dule.js gzip 39.4 kB 39.4 kB
Overall change 190 kB 190 kB
Client Pages
zeit/next.js canary zeit/next.js canary Change
_app.js 1.81 kB 1.81 kB
_app.js gzip 873 B 873 B
_error.js 12 kB 12 kB
_error.js gzip 4.73 kB 4.73 kB
hooks.js 12.7 kB 12.7 kB
hooks.js gzip 4.79 kB 4.79 kB
index.js 318 B 318 B
index.js gzip 222 B 222 B
link.js 8.14 kB 8.14 kB
link.js gzip 3.5 kB 3.5 kB
routerDirect.js 408 B 408 B
routerDirect.js gzip 281 B 281 B
withRouter.js 419 B 419 B
withRouter.js gzip 280 B 280 B
Overall change 35.8 kB 35.8 kB
Client Pages Modern
zeit/next.js canary zeit/next.js canary Change
_app.module.js 1.7 kB 1.7 kB
_app.module.js gzip 832 B 832 B
_error.module.js 23.3 kB 23.3 kB
_error.module.js gzip 8.59 kB 8.59 kB
hooks.module.js 1.52 kB 1.52 kB
hooks.module.js gzip 793 B 793 B
index.module.js 294 B 294 B
index.module.js gzip 223 B 223 B
link.module.js 8.53 kB 8.53 kB
link.module.js gzip 3.68 kB 3.68 kB
routerDirect.module.js 394 B 394 B
routerDirect..dule.js gzip 281 B 281 B
withRouter.module.js 404 B 404 B
withRouter.m..dule.js gzip 278 B 278 B
Overall change 36.1 kB 36.1 kB
Client Build Manifests
zeit/next.js canary zeit/next.js canary Change
_buildManifest.js 81 B 81 B
_buildManifest.js gzip 61 B 61 B
_buildManifest.module.js 81 B 81 B
_buildManife..dule.js gzip 61 B 61 B
Overall change 162 B 162 B
Serverless bundles
zeit/next.js canary zeit/next.js canary Change
_error.js 253 kB 253 kB
_error.js gzip 67.9 kB 67.9 kB
hooks.html 3.75 kB 3.75 kB
hooks.html gzip 979 B 979 B
index.js 254 kB 254 kB
index.js gzip 68.2 kB 68.2 kB
link.js 261 kB 261 kB
link.js gzip 70.3 kB 70.3 kB
routerDirect.js 255 kB 255 kB
routerDirect.js gzip 68.3 kB 68.3 kB
withRouter.js 254 kB 254 kB
withRouter.js gzip 68.4 kB 68.4 kB
Overall change 1.28 MB 1.28 MB

Commit: e9b2e25

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants