Warn if user is using <title> within the _document.js

[davscro]

resolves #4596
@timneutkens I'm not sure is it right place to call this function in render method or IMHO it's better to place it within the constructor, what do u think?

[timneutkens]

Same feedback as before: #4519 (comment)

[davscro]

but {(children || []).map((child) => { is not correct as children could be a single object not an array 😉

[timneutkens]

You can use https://reactjs.org/docs/react-api.html#reactchildrentoarray

[davscro]

Cool, I didn't know about React.Children utils, that's why I did custom implementation. Thanks for the hint.

[timneutkens]

I wonder if we can disable this in production / compile away the code

[davscro]

hei @timneutkens , I think so. We could update https://github.com/zeit/next.js/blob/673378e415bcb3bb33f8c3bb29ad49bf29778f4a/build/webpack.js#L261-L264 to have something like:

new webpack.DefinePlugin({
        'process.env.NODE_ENV': JSON.stringify(dev ? 'development' : 'production'),
        __DEV__: dev
      }),

so on the client scripts we could wrap codeblock with:

if(__DEV__){
   //our code which will not be included in the production bundle
}

[timneutkens]

There is no need for that, just use if(process.env.NODE_ENV === 'development')

[lucleray]

Am I missing something or this.props.children won't be rendered at all if NODE_ENV is production ? 😅

[timneutkens]

Correct, this is wrong, and uglify won't remove the block if it's not development. Instead we have to do something like this:

let children
if (process.env.NODE_ENV === 'development') {
 children = // etc etc
} else {
 children = this.props.children
}

This way uglify will remove the specific block

[lucleray]

Why not just use React.Children.forEach to show the warning ? And keep {this.props.children} ?

if (process.env.NODE_ENV === 'development') {
  React.Children.forEach(this.props.children, (child) => {
    if (child.type === 'title') {
      console.warn("...")
    }
  })
}
// return ...{this.props.children}...

[timneutkens]

Doing 1 loop instead of 2 🤔

[lucleray]

But only in development 🙈

[Jauny]

Random question - is it not better practice also to check process.env.NODE_ENV !== 'production' instead? I thought that it was safer to assume that 'production' would be present in prod. Some people might want warnings to show in 'staging' also for example.

[lucleray]

Yes ! That's also how it was implemented 🙂

#5160 (comment)