Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[CCA-Diversity] Webpack config #103

Closed
jvanbruegge opened this issue Apr 10, 2017 · 12 comments · May be fixed by #105
Closed

[CCA-Diversity] Webpack config #103

jvanbruegge opened this issue Apr 10, 2017 · 12 comments · May be fixed by #105
Assignees

Comments

@jvanbruegge
Copy link
Collaborator

jvanbruegge commented Apr 10, 2017

How to handle webpack configs:

  • Vanilla
    • Only one using webpack-merge
    • One for each environment (dev/prod) and language
  • webpack-blocks
    cc @nickbalestra
@jvanbruegge
Copy link
Collaborator Author

jvanbruegge commented Apr 10, 2017

I would vote for webpack-blocks, because it has a few advantages:

  • the config is divided into snippets (blocks) that can be composed, so having different configs for different languages is just a matter of composing the required blocks
  • we dont have to maintain the low level webpack config, we would outsource this part (kinda, I'm co-maintainer of webpack-blocks)
  • post-eject the config is really simple, short and readable. If you need custom behavior, you are not locked in. Creating custom blocks is easy.
  • It removes code duplication, we would could programmaticly generate a webpack config in setup.js

The only "disadvantage" is that we would introduce the blocks as devDependency

@nickbalestra
Copy link
Collaborator

Why I think that core-flavor shouldn't use webpack-block (and that's doesn't mean i don't see flavors being built with it, or webpack-blocks being bad, they are pretty awesome!)

  • CCA is made to hide complexity away, so mantaining the low-level part of that complexity is the CLI (our) job. I wouldn't therefore 'outsource' this to yet another project/dependency.
  • as a CCA mantainer I want to have full control over the configs used as I'm not an end user where this will solve my problems, but could potentially hide some source of errrors to me.
  • Such a dependency is hard to change down the road if we want to remove.
  • Post-Eject users (the only one who will be affected by this) should be the one to make such decision if they want to go with blocks, not us for them. The core flavor should be agnostic in this imho.
  • Code duplication is a non-problem for a couple of config files, when well documented actually the value that webpack configs files provide are much higher to people willing to learn how it work then an higher abstraction on top of it, at the end are simple configurations files..
  • I don't see really advantages in this case to have to introduce a devDependency with an higher abstraction over webpack configs when is our job to handle the low-level abstraction. I think we are not the right users for block

@jvanbruegge
Copy link
Collaborator Author

jvanbruegge commented Apr 10, 2017

  • because we hide it, it does not make a difference if we maintain it or another project
  • we will still have full control, The block config is completely referential transparent, so it's easy to swap a block with custom config. The blocks are designed in a way that they have sensible defaults, but they dont hide config if you want it
  • we would just have to copy + replace the block src with the block. Again they are transparent
  • we could even automate the copy + paste process and simply ask on eject
  • the problem is that we have redundant config, so if we want to add/cange something we dont just have one file to change, but allways all. This is a potential error source (typos, etc), about 90% of the configs are the same, and about 75% are common for every setup (create-react-app, normal WP app, boilerplates, etc).
  • the question is: Is it our job? We just have to be sure that the config works. That you can scaffold a cycle app with one command. How we do this should not matter.

@nickbalestra
Copy link
Collaborator

Hehe I can see you being very vocal about this, I don't have all that reluctancy for webpack configs but if you do I'm fine with either, will that make such a big difference? I'm not sure tbh...but if you believe so lets do it

@jvanbruegge
Copy link
Collaborator Author

yay
PR incoming :D

@jvanbruegge
Copy link
Collaborator Author

jvanbruegge commented Apr 10, 2017

How about .babelrc? I would add it to the template for two reasons:

  1. People want to customize which browsers should be supported (env-preset)
  2. For typescript I have to share the config, and it's easier this way

I would add it as extra file, and not to the package.json because that's not where people search for the config

@nickbalestra
Copy link
Collaborator

nickbalestra commented Apr 10, 2017

  1. we could simply have exposed a browser list on package.json but then this open the question to build/minify as I decided to use uglify meaning that babel will tell all its plugin to transpile to es5 in order not to brake it. Im not a big fan of babili, in quite few cases I've seen packages being way less optimal then simply doing a two pass ->es5 -> uglify. At that point setting the env-preset defeats the purpose...

  2. But if TS need this then I don't see anyother ways around...thoughts? I think you could have such file scoped and hidded within the vendor module (cycle-scrips) and that's how we should do it. No babelrc in the root please

@jvanbruegge
Copy link
Collaborator Author

  1. If we use uglify, we should not use the env-prefix
  2. i might be able to use the loader options

@nickbalestra
Copy link
Collaborator

  1. Why? You can use env preset with it just have to use the specific settings. Actually 'env' preset support not just browser lists but also env like 'es5' and thats' what happen behind the scene when you set 'uglify: true' in it the env preset options. And that's what babel recomend to use over es-2015. I noted it somewhere, I'll rather create a cycle-preset down the road and set it as per our like.

  2. perfect :)

@jvanbruegge
Copy link
Collaborator Author

ok

@nickbalestra
Copy link
Collaborator

As Diversity mean streamLib support, not related to language. Webpack.config doesn't need to be altered for the moment. This issue can be therefore closed

@nickbalestra
Copy link
Collaborator

Discussion grouped in #108

perjerz pushed a commit to perjerz/create-cycle-app that referenced this issue Nov 12, 2018
add cycle-github-emojis example application
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants