react-toolbox / react-toolbox-example

Example using react-toolbox
143 stars 97 forks source link

react-toolbox@2.0.0-beta.4, webpack@2.2.0 and react-hot-loader@3.0.0-beta.6 #41

Closed olegstepura closed 7 years ago

olegstepura commented 7 years ago

PR for react-toolbox@2.0.0-beta.4, webpack@2.2.0 and react-hot-loader@3.0.0-beta.6 see https://github.com/react-toolbox/react-toolbox-example/issues/32#issuecomment-274105918

Hot reloading works. there are a questions/issue that I could not fight with, but currently this does not prevent this from merging, it can be fixed later by anyone:

.button { background-color: var(--color-black); }

I get an error:

ERROR in ./~/css-loader?{"modules":true,"sourceMap":true,"importLoaders":1,"localIdentName":"[name]--[local]--[hash:base64:8]"}!./~/postcss-loader!./src/app/SuccessButton.css Module not found: Error: Can't resolve '../helpers.css' in 'C:\Temp\react-toolbox-example\src\app' @ ./~/css-loader?{"modules":true,"sourceMap":true,"importLoaders":1,"localIdentName":"[name]--[local]--[hash:base64:8]"}!./~/postcss-loader!./src/app/SuccessButton.css 3:10-237 10:48-275 15:80-307 16:76-303 17:84-311 19:80-307 @ ./src/app/SuccessButton.css @ ./src/app/SuccessButton.js @ ./src/app/App.js @ ./src/app/client.js @ multi (webpack)-dev-server/client?http://localhost:8080 webpack/hot/dev-server react-hot-loader/patch ./src/app/client.js


this issue seem to be caused by relative import in `composes:` keyword inside of the react-toolbox css file. Looks like it is tried to be resolved relative to the final file (`app/SuccessButton.css`), not initial, which has composes statement.
olegstepura commented 7 years ago

I also got rid of scss plugins and configs and got rid of server.js in favor of webpack-dev-server (less configuration to achive the same)

olegstepura commented 7 years ago

Initial comment also had this:

removed text - on final testing I realized that hot reloading stopped working (I'm sure it did). So I did this change in `client.js` ```diff - module.hot.accept('./App', render); + module.hot.accept(render); ```

But I fixed that with this change in webpack.config.js babel loader options:

-            ["es2015", { loose: true }],
+            ["es2015", { modules: false }],
olegstepura commented 7 years ago

@javivelasco I suggest you to merge this and to write some extra code inside this example to show how component customization is designed to be performed (using theme provider and per-component style customization).

BTW, webpack 2 is in release state here ;)

javivelasco commented 7 years ago

I've left some comments :) Good job

olegstepura commented 7 years ago

I've left answers to your comments. I need your further assistance to complete this PR.

olegstepura commented 7 years ago

I'm willing to get rid of semicolons in the line endings of *.js files. Many people do not use it since it's not required anymore. Also I suggest moving app/client.js -> frontend/index.js, www/index.html -> frontend/index.html and all components files to be moved from app/* -> frontend/component/* for better organization

Please let me know if you accept such changes.

javivelasco commented 7 years ago

Honestly I don't care too much about structure in this project. It is not supposed to serve as a boilerplate but to expose a Webpack configuration example. We should create in the future other examples for react-create-app if needed.

javivelasco commented 7 years ago

I don't care too much about it. This repository is intended to serve as an example of webpack configuration for react-toolbox, not as a boilerplate project. Also, react-toolbox works without this setup, maybe it's interesting to show a SSR example or something with CRA

olegstepura commented 7 years ago

I think SSR example should be a separate project, It'll be cool to see if react-toolbox can be used easily with webpack 2 and HMR without other complicated stuff (the same reason I got rid of server.js).

This one should demontrate how to use react-toolbox as a dependency, how to override component styling using themes and Theme Provider. That is it.

Do you know this feeling when you look at some boilerplate and it has all bells and whistles which you don't need at the moment. You take a look at the sources and find it's overcomplicated.

olegstepura commented 7 years ago

Ok, in my opinion PR is ready to be merged. @javivelasco you just need to show how to override style without !important rule in CSS.

javivelasco commented 7 years ago

Great, let me try it a bit more tomorrow and I will

Update: I'd rather use a custom express middleware like it was. What's the point of removing it? I think it easily delivers more control.

olegstepura commented 7 years ago

The point of removing custom middleware was to remove extra dependencies and extra configuration. It delivers more control, but except public IP it was not used. My point was to make example a simply configured setup which would allow one to use it as a base for his project.

If you insist, I can revert custom express middleware.

javivelasco commented 7 years ago

No, don't worry, I think that the less config the better. Still I need to try it but I have no time today, I'm doing it by tomorrow! Good! :)

olegstepura commented 7 years ago

Since there is airbnb code style config (.eslintrc), I edited code to fit it.

javivelasco commented 7 years ago

Cool, thanks! Couldn't check it yet btw, it's on my list. Patience! :)

GobbaFish commented 7 years ago

has anyone got the downloaded version for this configuration to work? I am getting the following errors

ERROR in ./~/react-toolbox/lib/checkbox/Check.js Module not found: Error: Can't resolve 'react-style-proptype' in 'C:\Downloads\react-toolbox-example\react-toolbox-example\node_modules\react-toolbox\lib\checkbox' @ ./~/react-toolbox/lib/checkbox/Check.js 15:26-57 @ ./~/react-toolbox/lib/checkbox/index.js @ ./~/react-toolbox/lib/index.js @ ./src/frontend/index.js @ multi (webpack)-dev-server/client?http://localhost:8080 webpack/hot/dev-server react-hot-loader/patch ./src/frontend/index.js

olegstepura commented 7 years ago

@GobbaFish this is fixed in https://github.com/react-toolbox/react-toolbox/pull/1190 but not deployed yet.

As a temporary workaround do npm i react-style-proptype (without --save)

GobbaFish commented 7 years ago

Thanks for the quick response. Btw, why without save?

olegstepura commented 7 years ago

@GobbaFish without save since it will be fixed in next release, so you will not need it if you do not use it in your project.

advance512 commented 7 years ago

Question, will this work with WebpackIsomorphicToolsPlugin?

advance512 commented 7 years ago

Ah, I see that it isn't SSR ready. Is there an SSR example available anywhere? I have been following tons of suggestions, guides and manuals (for Webpack 1.x, 2.x, postcss etc) and just can't get it to work.