react-webpack-generators / react-webpack-template

Simple react webpack template
MIT License
82 stars 43 forks source link

V2/V4 - template and install assume cssmodules: true #43

Closed sthzg closed 8 years ago

sthzg commented 8 years ago

I have just realized that the template's App.js and app.cssmodule.css implicitly assume a generator setup that will use css modules. When installing a V4 setup with prompting no for cssmodules, the dependency for "react-css-modules": "^3.7.6" is still present in package.json (thus it gets installed no matter what the user answers the prompt, and App.js will always have the @cssmodules annotation).

I see four paths from here:

  1. pulling out App.js/app.cssmodule.css from the template and into the generator
  2. defaulting App.js and app.css not to use css modules
  3. stop associating app.js with a style file at all
  4. keeping App.js and app.cssmodule.css as default in the template project and overriding it from the generator if necessary

I'd argument that 1 is the worst of these options, since the template would no longer work standalone, which I'd consider not being an option.

2, 3, and 4 should all be possible. 4 would require custom code (involving the deletion of app.cssmodule.css) and feel a bit unnatural, so at least as of now I think that I am pro 2 or 3.

@weblogixx what's your opinion, do I miss something more obvious?

weblogixx commented 8 years ago

Thats a good question. I think we should remove the cssmodules from App.js alltogether. It was nothing more as a demo to get started with anyways. We could add the react-css-modules package as an install step. What do you think?

sthzg commented 8 years ago

That's great, I also think it's the best decision. I will work on this asap.

For conditionally adding the dep to cssmodules I will create a separate issue on the generator repository.

sthzg commented 8 years ago

@weblogixx above's commit would be the changes required in this repo. Base.js still has the loader config for files with and without cssmodules, but that doesn't break anything and makes it easy for standalone users of the template to just install the dependency and start using css modules.

If you are fine with this I would merge and publish to start the work on the generator-side of this feature.

weblogixx commented 8 years ago

@sthzg, looks fine for me. Give it a go 👍

sthzg commented 8 years ago

Merged with https://github.com/react-webpack-generators/react-webpack-template/commit/59f0a2a25862977969517d332f8a797897ba0f14