Better default webpack config #1212

drazik · 2017-12-05T16:46:40Z

Related to #1210. It is currently a not tested WIP. I just want to get some feedback from @MoOx for the moment. Does it match to what you imagined ?

Here are some random things :

To filter the rules, I used the loader property, not test as you suggested to me, because some of the tests are arrays, so I felt it was more difficult to use it in conditions
I think some plugins used in react-scripts are not necessary in phenomic : InterpolateHtmlPlugin, HtmlWebpackPlugin, DefinePlugin... Do we keep them anyway ?

MoOx

Looking good so far! I like it!

MoOx · 2017-12-06T08:37:06Z

packages/plugin-bundler-webpack/package.json

@@ -33,6 +33,7 @@
    "find-cache-dir": "^0.1.1",
    "multili": "^1.1.0",
    "react-hot-loader": "^3.0.0-beta.7",
+    "react-scripts": "1.0.17",


Please use "^1.0.17"

MoOx · 2017-12-06T08:39:44Z

packages/plugin-bundler-webpack/src/webpack.config.js

@@ -3,22 +3,27 @@ import path from "path";
 import webpack from "webpack";
 import ExtractTextPlugin from "extract-text-webpack-plugin";

+const isStatic = process.env.PHENOMIC_ENV === "static";
+const isProduction = process.env.NODE_ENV === "production";


You cannot put this statically. Must be in the fonction to be safe (since we can adjust this during a function - we are not 100% sure that this file won't be imported (& so evaluated) before the code that will adjust this on runtime - FYI here and here)

MoOx · 2017-12-06T08:40:00Z

packages/plugin-bundler-webpack/src/webpack.config.js

+const isStatic = process.env.PHENOMIC_ENV === "static";
+const isProduction = process.env.NODE_ENV === "production";
+
+const baseConfig = require("react-scripts/config/webpack.config." +


Same as above, should be in the function.

MoOx · 2017-12-06T08:54:14Z

About the unnecessary plugins, no big deal imo, we can improve that later. I think we should only remove what's really a problem. I would like to stick to CRA default as close as possible so people can just "drop" some existing code. Removing some plugin might make sense but I prefer to not do it unless necessary.

MoOx · 2017-12-06T13:42:53Z

Any reason we have to maintain a specific css loader block? Is this because of the hmr?

drazik · 2017-12-06T13:50:11Z

@MoOx I did the changes you requested.

I had an error because the template for HtmlWebpackPlugin didn't exist. We don't need to use it in Phenomic, so I deleted the plugin

After that, I saw that the css was written in dist/static/css. It was because CRA config declares a cssFilename that is used in extractTextPluginOptions variable to set the publicPath in the loader. I added output.publicPath in our config, but I had to copy/paste the loader configuration without it.

I also had to put a weird computed property with a ternary condition in the CSS loader because CRA is using use in dev config and loader in prod config.

Last thing, since we have to override the ExtractTextPlugin used by CRA, and they use it only in prod, I prefered to delete it in every case and just add a new one with our own config.

I just tested with npm start and npm run build. The config does a good job on my blog.

EDIT : oops I didn't see your last comment. Actually it is because the config object is inside a ExtractTextPlugin.extract call, and we need to override this config. But I can't override it before it is processed by ExtractTextPlugin.extract, obviously. So I had to copy/paste the config and update small parts to make it fit our needs (HMR and publicPath)

MoOx · 2017-12-06T20:50:15Z

html plugin is supposed to be around https://github.com/facebookincubator/create-react-app/blob/4b55193f0ad479f26b0f5e153bb4b152a1ee03e7/packages/react-scripts/package.json#L45

Anyway, thanks for this, I will have a deeper look on some stuff before merging to try to reduce the maintenance even further. Thanks a lot for the work done. Stay tuned !

MoOx · 2017-12-06T20:51:36Z

Could you fix the yarn lock issue? Rebase on master and just do "yarn", yarn will handle the conflict (it's a yarn feature!) then you can amend your commit :)
Are you interested to be added as a collaborator on the project so you can later directly work on branches?

drazik · 2017-12-06T21:30:10Z

Yes the plugin is in the dependencies, but there is no HTML template file, so it crashed when it was trying to read it (CRA declares an index.html). Since we have nothing to do with such a file, I thought it was better to simply delete the plugin.

I will look the yarn lock issue tomorrow. And yeah, it would be so great to be added as a collaborator, thank you !

MoOx · 2017-12-18T16:35:54Z

@drazik any luck with yarn lock conflict? (sorry I think that's the second time??)

bloodyowl · 2017-12-18T17:36:14Z

@MoOx tkt je vais m'en occuper

MoOx · 2017-12-18T17:37:48Z

I can still do it (merge in another branch, resolve & the merge into master). What I expect from you is more a code review what thoughts about the approach :)

bloodyowl

the fact that its a bit too much react centric is kind of annoying I think, where should we branch? every renderer adding their options given the bundler or the other way around?

bloodyowl · 2017-12-18T17:40:09Z

packages/plugin-bundler-webpack/src/webpack.config.js

+  const isStatic = process.env.PHENOMIC_ENV === "static";
+  const isProduction = process.env.NODE_ENV === "production";
+
+  const baseConfig = require("react-scripts/config/webpack.config." +


I'm a bit worried to rely on a react repo for the webpack config (they are not coupled in Phenomic)

bloodyowl · 2017-12-18T17:40:32Z

packages/plugin-bundler-webpack/src/webpack.config.js

+    entry: {
+      [config.bundleName]: [
+        !isStatic && require.resolve("webpack-hot-middleware/client"),
+        !isStatic && require.resolve("react-hot-loader/patch"),


I'd only add this if the React plugin is used

MoOx · 2017-12-18T17:51:11Z

Totally agree. I was more like "until we have other renderer we could do this and fix later".
The correct approach would me more like that any plugin could send option to any other plugins (each plugins can accept options (in theory), we could add an api or something so plugin-renderer-react could pass a config to plugin-bundler-webpack somehow...

drazik · 2017-12-19T11:31:15Z

I fixed the conflict in yarn.lock

MoOx · 2017-12-20T21:40:00Z

It seems that tests are failing on Windows (see appveyor state). Can you take a look?

drazik · 2018-01-02T08:39:22Z

Yes I will take a look. I'm pretty busy at the moment but I will take 10 minutes to do it this week ;)

Edit :

The error is :

ModuleParseError: Module parse failed: C:\projects\phenomic\examples\react-app-getting-started\App.js Unexpected token (14:2)
You may need an appropriate loader to handle this file type.

It seems the babel-loader is not used. But I can't see why it's the case only on Windows. Since I only have a machine with Fedora, I can't really test things directly. Do you see what can go wrong ?

MoOx · 2018-01-22T12:58:13Z

@drazik would you like me to continue the effort here?

drazik · 2018-01-23T08:05:29Z

Yes sorry I am very busy these days, and I don't know where the error on Windows come from. If you have time to investigate it's great :)

MoOx · 2018-03-15T05:59:31Z

I will try to use another approach soon. @drazik thanks a lot for digging into this, that helped me to think about a better way to integrate CRA config!

drazik · 2018-03-16T21:44:21Z

That's fine :) In the meantime, do you have some advices to deal with things the current webpack things doesn't handle ? I have some fonts to load, for example.

… custom webpack conf see MoOx/phenomic#1212

BREAKING CHANGE * move old content in a folder * add articles * use phenomic with basic components * add gitignore * manage external posts * max 80 chars * outdated message * delete twitter share link * Delete PostsList page to show all posts on home * use named exports only * code highlight * set language on all code samples * add error page * move posts images to public folder * add progress bar loading * handle fonts * chore: update dependencies * feat: disable custom fonts until I find how to deal with phenomic and custom webpack conf see MoOx/phenomic#1212 * feat: update header with new job * feat: add emoji on external links * content: add preact discover post * chore: delete travis deploy script

MoOx · 2018-03-26T06:24:45Z

You can create your own custom config. Eg: https://github.com/MoOx/maxim-hylian.fr/blob/master/webpack.config.js

drazik · 2018-03-27T08:09:44Z

Thank you 👍

MoOx suggested changes Dec 6, 2017

View reviewed changes

MoOx changed the title ~~[WIP] Better default webpack config~~ Better default webpack config Dec 6, 2017

drazik force-pushed the improve-webpack-config branch from e26dfe2 to a47714d Compare December 7, 2017 09:45

MoOx assigned MoOx and bloodyowl Dec 18, 2017

bloodyowl reviewed Dec 18, 2017

View reviewed changes

drazik added 9 commits December 19, 2017 12:30

webpack config based on react-scripts

d140a03

fix plugins map

5ef5490

add caret to react-scripts version in package.json

635a5e5

put isStatic, isProduction and baseConfig in the webpack config function

47cd5bd

delete HtmlWebpackPlugin from plugins list

460d7de

override ExtractTextPlugin loader options

4857c96

add output.publicPath

8d5afea

fix publicPath typo

ca81278

fix problem with ExtractTextPlugin

06219a2

drazik force-pushed the improve-webpack-config branch from a47714d to 06219a2 Compare December 19, 2017 11:30

MoOx closed this Mar 15, 2018

drazik added a commit to drazik/blog that referenced this pull request Mar 16, 2018

feat: disable custom fonts until I find how to deal with phenomic and…

c73f77c

… custom webpack conf see MoOx/phenomic#1212

drazik added a commit to drazik/blog that referenced this pull request Mar 16, 2018

feat: disable custom fonts until I find how to deal with phenomic and…

099aa07

… custom webpack conf see MoOx/phenomic#1212

drazik deleted the improve-webpack-config branch March 3, 2019 22:02

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Better default webpack config #1212

Better default webpack config #1212

drazik commented Dec 5, 2017 •

edited

Loading

MoOx left a comment

MoOx Dec 6, 2017

MoOx Dec 6, 2017

MoOx Dec 6, 2017

MoOx commented Dec 6, 2017

MoOx commented Dec 6, 2017

drazik commented Dec 6, 2017 •

edited

Loading

MoOx commented Dec 6, 2017

MoOx commented Dec 6, 2017

drazik commented Dec 6, 2017

MoOx commented Dec 18, 2017

bloodyowl commented Dec 18, 2017

MoOx commented Dec 18, 2017

bloodyowl left a comment

bloodyowl Dec 18, 2017

bloodyowl Dec 18, 2017

MoOx commented Dec 18, 2017

drazik commented Dec 19, 2017

MoOx commented Dec 20, 2017

drazik commented Jan 2, 2018 •

edited

Loading

MoOx commented Jan 22, 2018

drazik commented Jan 23, 2018

MoOx commented Mar 15, 2018

drazik commented Mar 16, 2018

MoOx commented Mar 26, 2018

drazik commented Mar 27, 2018

Better default webpack config #1212

Better default webpack config #1212

Conversation

drazik commented Dec 5, 2017 • edited Loading

MoOx left a comment

Choose a reason for hiding this comment

MoOx Dec 6, 2017

Choose a reason for hiding this comment

MoOx Dec 6, 2017

Choose a reason for hiding this comment

MoOx Dec 6, 2017

Choose a reason for hiding this comment

MoOx commented Dec 6, 2017

MoOx commented Dec 6, 2017

drazik commented Dec 6, 2017 • edited Loading

MoOx commented Dec 6, 2017

MoOx commented Dec 6, 2017

drazik commented Dec 6, 2017

MoOx commented Dec 18, 2017

bloodyowl commented Dec 18, 2017

MoOx commented Dec 18, 2017

bloodyowl left a comment

Choose a reason for hiding this comment

bloodyowl Dec 18, 2017

Choose a reason for hiding this comment

bloodyowl Dec 18, 2017

Choose a reason for hiding this comment

MoOx commented Dec 18, 2017

drazik commented Dec 19, 2017

MoOx commented Dec 20, 2017

drazik commented Jan 2, 2018 • edited Loading

MoOx commented Jan 22, 2018

drazik commented Jan 23, 2018

MoOx commented Mar 15, 2018

drazik commented Mar 16, 2018

MoOx commented Mar 26, 2018

drazik commented Mar 27, 2018

drazik commented Dec 5, 2017 •

edited

Loading

drazik commented Dec 6, 2017 •

edited

Loading

drazik commented Jan 2, 2018 •

edited

Loading