survivejs / webpack-book

From apprentice to master (CC BY-NC-ND)
https://survivejs.com/webpack/
2.42k stars 319 forks source link

Configuring react chapter - align one code listing in accordance with previous chapters and typos #154

Closed jeggett closed 7 years ago

jeggett commented 7 years ago
  1. Section Rendering a React application. Code listing app/index.js has completely new text in comparison with Minifying the build chapter. I think it's good idea to add line-through lines, etc. so reader know how to change his config while he read through the book.
  2. Section 'Configuring with Babel'. Sentence 'To enable JSX with Babel, an addition preset is required'. I think you mean additional.
  3. Section 'Configuring with Webpack'. Sentence Instead of matching only /\.js$/, we can expand it to include .jsx extension through /\.(js|jsx)$/. I think 'against' is needed after 'only', because we match strings against regexps.
  4. In the chapter you capitalize Babel, but Webpack always starts from lowercase letter (everywhere except one section header). Maybe capitalize Webpack everywhere in the book except code listings so everything looks consistent?
bebraw commented 7 years ago
  1. Section Rendering a React application. Code listing app/index.js has completely new text in comparison with Minifying the build chapter. I think it's good idea to add line-through lines, etc. so reader know how to change his config while he read through the book.

This is a tough one. I planned it as a simple stand-alone example. I could change the text to reflect this.

  1. & 3.

Fixed.

In the chapter you capitalize Babel, but Webpack always starts from lowercase letter (everywhere except one section header). Maybe capitalize Webpack everywhere in the book except code listings so everything looks consistent?

Officially webpack is spelled in lowercase. I ended up with an uneasy compromise where webpack is capitalized when it's in the beginning of a sentence and lowercase otherwise. This is more consistent with the common usage. Pushing all instances of webpack to lowercase would be one way I suppose. I'm not so sure about this.

jeggett commented 7 years ago

About 1: Currently we don't have inline: true so we implement HMR inteface ourselves. The question is how to implement this interface with react. So only changed parts of code and not whole dom-tree updated on change.

About 4: I see. So lets just make 'Webpack' in one section header lowercase as in 'Bundle with webpack.' caption on webpack.js.org

bebraw commented 7 years ago

About 1: Currently we don't have inline: true so we implement HMR inteface ourselves. The question is how to implement this interface with react. So only changed parts of code and not whole dom-tree updated on change.

Maybe the best option would be to drop the React HMR appendix and show how to achieve the setup with react-hot-loader 3 right here? Then you can see how the client portion would have to change.

One more option would be to do both. Show a small example without HMR (pretty much the current example) and then explain after that how to expand the project and show the full diff.

About 4: I see. So lets just make 'Webpack' in one section header lowercase as in 'Bundle with webpack.' caption on webpack.js.org

I follow titlecase for titles and that would be pretty big change (lots of other places to check too). I think I'll leave it as is for now as it looks good enough. 😄

jeggett commented 7 years ago

The key moment is

show how to achieve the setup with react-hot-loader 3 right here

If I were you I would merge 'Configuring Hot Module Replacement with React' into 'Configuring React' and complement it with react-hot-loader setup. The goal is to show most development friendly setup and explain what's happening under the hood at least conceptually.

bebraw commented 7 years ago

If I were you I would merge 'Configuring Hot Module Replacement with React' into 'Configuring React' and complement it with react-hot-loader setup. The goal is to show most development friendly setup and explain what's happening under the hood at least conceptually.

Yeah, I'll give it a go. Thanks. 👍