Improve config documentation and remove 22 dependencies from cssnano #1168
Conversation
| @@ -24,7 +24,8 @@ | |||
| ], | |||
| "license": "MIT", | |||
| "dependencies": { | |||
| "cosmiconfig": "^7.0.0", | |||
| "lilconfig": "^2.0.3", | |||
| "yaml": "^1.10.2", | |||
There was a problem hiding this comment.
I was uncertain whether to keep YAML config parsing since it was never documented explicitly, but removing would be a breaking change in theory.
There was a problem hiding this comment.
Sounds like a good call to keep it for now, even if the chance of it being used is remote 👍
Question:
Since the tests you added were paired with a .cssnanorc.json and you mentioned keeping YAML support, I decided to attempt running the test suite with a .cssnanorc.yaml and a few other variations (replacing .cssnanorc.json in the config_loading dir) for the sake of diligence. And it doesn't appear to be picked up.
It's entirely possible I'm having some form of "brainfart" and doing something wrong here - but shouldn't that work?
(Out of curiosity I had a look at the searchItems found by lilconfig in lilconfigSync.search() and it only appears to look for package.json, cssnanorc.json, cssnano.config.js, and cssnanorc.js, .cssnanorc.cjs, and cssnano.config.cjs)
There was a problem hiding this comment.
My inner conviction is nobody is really using the YAML format, so I did not bother checking whether it works, I have just copied the instructions on the lilconfig repo. Looking at the lilconfig code, it seems it does not even load .rc (with no extension) by default.
| @@ -85,7 +120,3 @@ These plugins will run after once all presets operations are complete. | |||
| } | |||
| ``` | |||
|
|
|||
| ## Alternatives | |||
There was a problem hiding this comment.
The section about the PostCSS config file has been moved to the top as that is what most people will use and the PostCSS config overrides the config in the cssnano files.
Codecov Report
@@ Coverage Diff @@
## master #1168 +/- ##
==========================================
- Coverage 96.53% 96.51% -0.03%
==========================================
Files 114 114
Lines 3582 3584 +2
Branches 1052 1052
==========================================
+ Hits 3458 3459 +1
- Misses 115 117 +2
+ Partials 9 8 -1
Continue to review full report at Codecov.
|
Closes #904 Add tests for configuration loading and correct the documentation. * correct configuration file names in the docs * specify that the PostCss config overrides the cssnano-specific config * remove mention of cosmiconfig to avoid being tied to a single implementation
ludofischer
force-pushed
the
replace-config-loading
branch
from
81dc41f
to
a1db6ff
Compare
| loaders: { | ||
| '.yaml': (filepath, content) => yaml.parse(content), | ||
| '.yml': (filepath, content) => yaml.parse(content), | ||
| }, |
There was a problem hiding this comment.
I wonder if you may also need to add something like searchPlaces: ['.cssnanorc.yaml', '.cssnanorc.yml'] to the options in order for lilconfig to actually look for yaml files?
It seems like lilconfig uses this as basis for merging in the loaders.
Ref. #1168 (comment)
There was a problem hiding this comment.
Seems like a bit of a bug or design weakness on their part
There was a problem hiding this comment.
I've updated the code with explicit search places. I did try renaming the file to .cssnanorc.yml and it worked.
There was a problem hiding this comment.
Confirmed! And perhaps not such a bad thing for it to be explicit anyway 👍
Remove about 22 dependencies according to npm i --production (from 110 to 88 deps) * reduce node_modules after `yarn install cssnano` in empty project from 14MB to 13MB * add YAML dep to preserve YAML configuration compatibility YAML configuration was only implicitly documented by referring to cosmiconfig, but removing it would be a breaking change
ludofischer
force-pushed
the
replace-config-loading
branch
from
a1db6ff
to
8e39090
Compare
|
/cc @sigveio I can invite your in cssnano repo so you can help us more better 😄 @ludofischer What do you think? |
Sure, @sigveio can certainly help. |
|
Thanks @alexander-akait and @ludofischer - I appreciate the trust, and looking forward to help where I can 😄 |
This PR does two things.
First, I've reworked the documentation to mention the config file priorities and added some tests to verify the basic config loading.
In a separate commit, I've replaced cosmiconfig with lilconfig. This gets rid of 22 dependencies and about 1MB when doing
npm i cssnano. This makes cssnano use the same library as the lastest postcss-load-config (postcss/postcss-load-config@d147864).You can't see the removed dependencies in the repository
yarn.lockbecause lerna depends on cosmiconfig.