Allow no-console development
#2008
Conversation
…ag is set on `yarn build`, in which case it will throw an error.
|
|
…tion.json` as per suggestion from @swalkinshaw: roots#2008 (comment)
|
If you make it |
|
Yeah I did wonder that if we're adding an actual file, then maybe all the others should be moved from |
|
IMO I'd lead toward two files (i.e. |
|
I think taking that logic to the extreme would make things more confusing, especially cuz if you wanted to add the rule to both, you now have to add it in two places, which is going to be the more likely update. Generally speaking, throughout Sage, you have single files with conditionals based on the env (e.g. the webpack build has this, and I think that's more common and easier to understand. As a comparison, I just started a project with Create React App and ejected, and found two separate webpack configurations, and I kinda hate it. Once I start tinkering around in there, I feel like I'm doing everything twice (not to mention it feels kind of over engineered for my purposes, but that's a separate issue). |
|
One of Sage's goals is to keep dev and prod as close as possible. Using a single file would probably encourage this. Right now there'd only be a single difference (debugger related things). |
…nditionals in the config file itself, significantly simpliying rule creation. For small-scale differences, this seems to be the simplest solution.
|
Solid arguments, my bad for not digging a bit deeper initially. Setting conditionals inside |
resources/assets/build/config.js
Outdated
| @@ -20,6 +20,7 @@ const config = merge({ | |||
| root: rootPath, | |||
| assets: path.join(rootPath, 'resources/assets'), | |||
| dist: path.join(rootPath, 'dist'), | |||
| eslintProd: path.join(rootPath, '.eslintrc.production.json'), | |||
* upstream/master: Don't need this any more! :( This moves our eslint config in .eslint.js, which allows us to use conditionals in the config file itself, significantly simpliying rule creation. For small-scale differences, this seems to be the simplest solution. Rename production eslint file from `.eslintrcProd` to `.eslint.production.json` as per suggestion from @swalkinshaw: roots#2008 (comment) Build process now allows console.log, except when the `production` flag is set on `yarn build`, in which case it will throw an error. Delete .eslintrc Remove some Bower traces Update README.md Add Code of Conduct [ci skip] Change .dev to .test for default local dev TLD per roots/trellis#923 Bootstrap function in _variables.scss to play nice with other frameworks Move variables and Bootstrap imports to autoload Remove explicit Bootstrap lines Example $theme-color Update main.scss Update controller examples Update dependencies Update CHANGELOG [ci skip] Bootstrap 4 Beta 2 Remove get_the_posts_navigation from 404
* sage/master: Updated bootstrap to release version 4.0.0 Updated popper.js to version 1.12.9 Add php-mbstring to list of requirements Don't need this any more! :( This moves our eslint config in .eslint.js, which allows us to use conditionals in the config file itself, significantly simpliying rule creation. For small-scale differences, this seems to be the simplest solution. Rename production eslint file from `.eslintrcProd` to `.eslint.production.json` as per suggestion from @swalkinshaw: roots#2008 (comment) Build process now allows console.log, except when the `production` flag is set on `yarn build`, in which case it will throw an error. Delete .eslintrc Remove some Bower traces Update README.md Add Code of Conduct [ci skip] Change .dev to .test for default local dev TLD per roots/trellis#923 Bootstrap function in _variables.scss to play nice with other frameworks Move variables and Bootstrap imports to autoload Remove explicit Bootstrap lines Example $theme-color Update main.scss Update controller examples
* master: (339 commits) Update CHANGELOG.md 9.0.0 Update README [ci skip] Update CHANGELOG [ci skip] npm lockfile Update dependencies Updated bootstrap to release version 4.0.0 Updated popper.js to version 1.12.9 Add php-mbstring to list of requirements Don't need this any more! :( This moves our eslint config in .eslint.js, which allows us to use conditionals in the config file itself, significantly simpliying rule creation. For small-scale differences, this seems to be the simplest solution. Rename production eslint file from `.eslintrcProd` to `.eslint.production.json` as per suggestion from @swalkinshaw: roots#2008 (comment) Build process now allows console.log, except when the `production` flag is set on `yarn build`, in which case it will throw an error. Delete .eslintrc Remove some Bower traces Update README.md Add Code of Conduct [ci skip] Change .dev to .test for default local dev TLD per roots/trellis#923 Bootstrap function in _variables.scss to play nice with other frameworks Move variables and Bootstrap imports to autoload Remove explicit Bootstrap lines ...
So far as I can tell, .eslint files don't allow for internal conditionals, so this PR implements a "production" eslint file that is used when the production flag is set. Otherwise, it uses
package.json. I structured it this way so that text editors will hopefully not be confused about which eslint file to read rules from. Rules can also be directly passed in the webpack config, but I felt that this would scale poorly and a separate file makes things a bit easier to understand. This change would also allow for easy customization for other dev vs prod eslint rules.This would resolve issue #2006.