Feat: postcss lowercase text by anikethsaha · Pull Request #867 · cssnano/cssnano

anikethsaha · 2020-01-16T05:54:37Z

Moved postcss-lowercase-text to cssnano-preset-default
Updated few integration tests.

codecov-io · 2020-01-16T05:58:53Z

Codecov Report

Merging #867 into master will increase coverage by 0.00%.
The diff coverage is 97.53%.

@@           Coverage Diff           @@
##           master     #867   +/-   ##
=======================================
  Coverage   97.28%   97.28%           
=======================================
  Files         118      123    +5     
  Lines        3458     3539   +81     
  Branches     1040     1054   +14     
=======================================
+ Hits         3364     3443   +79     
- Misses         86       88    +2     
  Partials        8        8

Impacted Files	Coverage Δ
packages/cssnano-preset-default/src/index.js	`100.00% <ø> (ø)`
packages/postcss-lowercase-text/src/atRules.js	`88.23% <88.23%> (ø)`
...wercase-text/src/__tests__/data/pseudo-elements.js	`100.00% <100.00%> (ø)`
packages/postcss-lowercase-text/src/index.js	`100.00% <100.00%> (ø)`
packages/postcss-lowercase-text/src/selector.js	`100.00% <100.00%> (ø)`
packages/postcss-lowercase-text/src/units.js	`100.00% <100.00%> (ø)`
... and 1 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8dd5c44...da03b82. Read the comment docs.

anikethsaha · 2020-01-28T08:00:19Z

cc @evilebottnawi

packages/postcss-lowercase-text/src/__tests__/data/pseudo-elements.js

packages/postcss-lowercase-text/src/__tests__/atRules.test.js

alexander-akait

Looks good, we need more improvement - let's do it

packages/postcss-lowercase-text/package.json

packages/cssnano-preset-default/package.json

packages/postcss-lowercase-text/README.md

alexander-akait

Can we add more tests?

packages/postcss-lowercase-text/src/__tests__/atRules.test.js

anikethsaha · 2020-02-04T10:37:41Z

packages/postcss-lowercase-text/src/__tests__/index.test.js

+  run(
+    'should not transform css variable declarations',
+    ':root {  --BG-COLOR: coral; --TEXT-COLOR: RED;}',
+    ':root {  --BG-COLOR: coral; --TEXT-COLOR: red;}'


they are css values, they can be transformed as well just like the rest of the values.

Like this

https://github.com/cssnano/cssnano/pull/867/files#diff-fe88a57283a0a1a5c792a3000e5badbaR43-R53

I will read spec, maybe you are right

anikethsaha · 2020-02-04T10:58:58Z

@evilebottnawi Can you please check this allowed transformation

 [at rule]            [allowed lowercasing]
  keyframes -> nameAndPropsTransform,
  counter-style  -> namePropsTransform,
  namespace  ->  nameTransformOnly,
  import  ->  nameTransformOnly,
  font-face  ->   nameAndPropsTransform,
  font-feature-values   ->  nameTransformOnly,
  page   ->  nameAndPropsTransform,
  supports   ->   nameAndPropsTransform,
  media  ->   nameParamsPropsTransform,
  charset  ->  nameTransformOnly,
  document  ->   nameTransformOnly,
  viewport  ->   nameAndPropsTransform,

alexander-akait · 2020-03-24T10:19:39Z

Need fix CI

anikethsaha · 2020-03-24T10:21:35Z

Need fix CI

working on it

anikethsaha · 2020-04-27T08:13:04Z

cc @evilebottnawi finally the CI is green. Can you review this.
we need to rebase other PR in order to fix their CI as there is some issue with @babel/preset-env and node13 for which I had to update it to 7.9 so after merging this, we need to rebase other PR.

alexander-akait · 2020-04-28T10:27:15Z

jest.config.js

    '_processCSS.js',
    '_processCss.js',
    '_webpack.config.js',
+    'packages/postcss-lowercase-text/src/__tests__/data/',


Why it here?

jest was running the test running in this directory as well as there is an .js file which is true for jest's default test file pattern. So I added them. this directory should not be tested as it has data only.

alexander-akait · 2020-04-28T10:28:03Z

packages/cssnano-preset-default/package.json

    "postcss-discard-duplicates": "^4.0.2",
    "postcss-discard-empty": "^4.0.1",
    "postcss-discard-overridden": "^4.0.1",
+    "postcss-lowercase-text": "*",


Use 1.0.0 verison

I didnt publish v1 . so it will through error if we build it locally. I will release it manually after merging of this.

alexander-akait · 2020-04-28T10:28:19Z

packages/cssnano-util-get-match/package.json

  },
  "engines": {
-    "node": ">=10.13"
+    "node": ">=10.13.0"


No need .0

alexander-akait · 2020-04-28T10:29:08Z

packages/cssnano/src/__tests__/postcss-calc.js

@@ -1,5 +1,7 @@
 import processCss from './_processCss';

+jest.setTimeout(30000);


Avoid it and use setupFilesAfterEnv https://jestjs.io/docs/en/jest-object#jestsettimeouttimeout

ok cool, I will try this 👍

alexander-akait · 2020-04-28T10:30:04Z

packages/postcss-lowercase-text/src/__tests__/atRules.test.js

+    '@MEDIA SCREEN and (min-width: 480px){}',
+    '@media screen and (min-width: 480px){}'
+  );
+});


Please add test: @MEDIA SCREEN AND (MIN-WIDTH: 480PX) {}

ok cool 👍

alexander-akait · 2020-04-28T10:31:44Z

packages/postcss-lowercase-text/src/__tests__/atRules.test.js

+  run(
+    'should safely transform the @charset',
+    '@CHARSET "iso-8859-15";',
+    '@charset "iso-8859-15";'


Do not touch charset never, it should b always in lowercase, we should keep @CHARSET as is

is it case sensitive?

alexander-akait · 2020-04-28T10:32:10Z

packages/postcss-lowercase-text/src/__tests__/atRules.test.js

+    'should  transform the @import ',
+    `@IMPORT url("fineprint.css") print;@import url("bluish.css") speech;@ImPoRt 'custom.css';@import url("CHROME://communicator/SKIN/");@import "common.css" screen;`,
+    `@import url("fineprint.css") print;@import url("bluish.css") speech;@import 'custom.css';@import url("CHROME://communicator/SKIN/");@import "common.css" screen;`
+  );


Please add test: @IMPORT URL("fineprint.css") PRINT;

ok cool 👍

alexander-akait · 2020-04-28T10:33:19Z

packages/postcss-lowercase-text/src/__tests__/atRules.test.js

+  run(
+    'should safely transform the @namespace',
+    '@NAMESPACE prefix url(http://www.w3.org/1999/xhtml);',
+    '@namespace prefix url(http://www.w3.org/1999/xhtml);'


Please add test @NAMESPACE SVG URL(http://www.w3.org/2000/svg);, SVG should be in uppercase (keep as is)

alexander-akait · 2020-04-28T10:33:42Z

packages/postcss-lowercase-text/src/__tests__/atRules.test.js

+  run(
+    'should safely transform the @supports ',
+    '@SUPPORTS (display: grid) {div {display: grid;}};@supports not (display: GRID) {div {float: right;}}',
+    '@supports (display: grid) {div {display: grid;}};@supports not (display: GRID) {div {float: right;}}'


Please add test: @SUPPORTS (DISPLAY: GRID) {}

alexander-akait · 2020-04-28T10:34:44Z

packages/postcss-lowercase-text/src/__tests__/atRules.test.js

+  run(
+    'should safely transform the @document  ',
+    '@DOCUMENT URL("https://www.EXAMPLE.com/") {h1 {COLOR: green;}}',
+    '@document URL("https://www.EXAMPLE.com/") {h1 {color: green;}}'


URL should be lowercased

@evilebottnawi, it will lowercase the argument ("https://www.EXAMPLE.com/") as well which is not saved as lowercasing anything after domain name in URLs are not safe.
postcss will parse URL("https://www.EXAMPLE.com/") whole as params so we cant lowercase only URL.

let me know what you think!

alexander-akait · 2020-04-28T10:35:16Z

packages/postcss-lowercase-text/src/__tests__/atRules.test.js

+  run(
+    'should safely transform the @page   ',
+    '@PAGE {margin: 1cm;};@page :FIRST {margin: 2cm;}',
+    '@page {margin: 1cm;};@page :FIRST {margin: 2cm;}'


Please add test: @PAGE :FIRST (all should be lowercased)

alexander-akait · 2020-04-28T10:36:17Z

packages/postcss-lowercase-text/src/__tests__/atRules.test.js

+  run(
+    'should safely transform the @keyframes',
+    '@KEYFRAMES slidein {from {transform: translateX(0%);}};@KEYFRAMES important2  {FROM {TRANSFORM: translateX(0%);}}',
+    '@keyframes slidein {from {transform: translateX(0%);}};@keyframes important2  {from {transform: translateX(0%);}}'


Please add test - @KEYFRAMES SLIDEIN {}, SLIDEIN should not be lowercased (keep as is)

alexander-akait · 2020-04-28T10:38:30Z

packages/postcss-lowercase-text/src/__tests__/data/pseudo-elements.js

+  },
+};
+
+export default pseudoElementsList;


Can we write a script and get it from mdn?

I don't know of any API to request from MDN !

can you share any link.

alexander-akait · 2020-04-28T10:41:16Z

packages/postcss-lowercase-text/src/__tests__/index.test.js

+  run(
+    'should safely transform the  CSS attribute Selectors to lowercase',
+    '[NAME="newname"]{border: 1px solid black;}',
+    '[name="newname"]{border: 1px solid black;}'


Please add tests:

A[HREF*="insensitive" I] -> a[href*="insensitive" i]

A[HREF*="insensitive" S] -> a[href*="insensitive" s]

alexander-akait · 2020-04-28T10:41:44Z

packages/postcss-lowercase-text/src/__tests__/index.test.js

+  run(
+    'should safely transform the  CSS attribute Selectors to lowercase',
+    '[NAME="newname"]{border: 1px solid black;}',
+    '[name="newname"]{border: 1px solid black;}'


Please add test: DIV:NOT([LANG])

alexander-akait · 2020-04-28T10:43:36Z

packages/postcss-lowercase-text/src/__tests__/index.test.js

+    'should safely transform the  CSS attribute Selectors to lowercase',
+    '[NAME="newname"]{border: 1px solid black;}',
+    '[name="newname"]{border: 1px solid black;}'
+  );


Please add test: LI[DATA-VALUE|="1900"], we should not lowercase DATA-VALUE (keep as is) because https://html.spec.whatwg.org/multipage/dom.html#embedding-custom-non-visible-data-with-the-data-*-attributes

alexander-akait · 2020-04-28T10:44:29Z

packages/postcss-lowercase-text/src/__tests__/index.test.js

+  run(
+    'should not transform the  CSS :active pseudo class Selectors to lowercase',
+    'a:ACTIVE{border: 1px solid black;}',
+    'a:ACTIVE{border: 1px solid black;}'


Why we do not lowercased?

alexander-akait · 2020-04-28T10:45:02Z

packages/postcss-lowercase-text/src/__tests__/index.test.js

+  );
+
+  run(
+    'should safely transform the  CSS nested classname  Selectors to lowercase',


Bad name test

alexander-akait · 2020-04-28T10:45:24Z

packages/postcss-lowercase-text/src/__tests__/index.test.js

+describe('transforming CSS units', () => {
+  run(
+    'should safely transform the absolute units to lowercase : px',
+    'classname{border: 1PX solid black;}',


change classname to .classname

alexander-akait · 2020-04-28T10:46:50Z

packages/postcss-lowercase-text/src/__tests__/index.test.js

+  run(
+    'should safely transform Property ',
+    ':root {  --main-BG-color: coral;} .classname {   color : RED;  background-color: var(--main-BG-color);}',
+    ':root {  --main-BG-color: coral;} .classname {   color : red;  background-color: var(--main-BG-color);}'


Please add test - background-color: VAR(--MY-VAR, VAR(--MY-BACKGROUND, PING));

should PING be lowercased too ?

alexander-akait · 2020-04-28T10:48:19Z

packages/postcss-lowercase-text/src/index.js

+    let allAtRulesNames = []; // ignored list for transforming values
+    css.walkAtRules((rule) => {
+      const allowedTransformingTypes = Object.keys(atRulesTranformerMap);
+      const allowedTransformingTypeSet = new Set(allowedTransformingTypes);


No need initialize inside walk, it is reduce perf

alexander-akait · 2020-04-28T10:48:55Z

packages/postcss-lowercase-text/src/index.js

+          decl.prop = decl.prop.toLowerCase();
+        }
+        // font-family is case sensitive prop, its value should be left as it is
+        if (decl.prop === 'font-family') {


We have many properties which we should lowercased

can you name them ? may all font as well ?

alexander-akait · 2020-04-28T10:49:25Z

packages/postcss-lowercase-text/src/index.js

+        decl.value = valueParser(decl.value)
+          .walk((node) => {
+            i += 1;
+            if (node.type === 'word' && i === 1) {


Avoid using counter, need rewrite

anikethsaha requested a review from alexander-akait January 16, 2020 06:01

anikethsaha mentioned this pull request Jan 28, 2020

mergeLonghand lowercases values with multiple var references #847

Closed

alexander-akait reviewed Jan 28, 2020

View reviewed changes

packages/postcss-lowercase-text/src/__tests__/data/pseudo-elements.js Outdated Show resolved Hide resolved

alexander-akait reviewed Jan 28, 2020

View reviewed changes

packages/postcss-lowercase-text/src/__tests__/atRules.test.js Outdated Show resolved Hide resolved

alexander-akait reviewed Jan 28, 2020

View reviewed changes

packages/postcss-lowercase-text/package.json Outdated Show resolved Hide resolved

anikethsaha mentioned this pull request Jan 28, 2020

[Breaking]: dropping node 6.9 to >=10.13.0 #871

Merged

alexander-akait reviewed Jan 30, 2020

View reviewed changes

packages/cssnano-preset-default/package.json Outdated Show resolved Hide resolved

alexander-akait reviewed Jan 30, 2020

View reviewed changes

packages/postcss-lowercase-text/README.md Outdated Show resolved Hide resolved

alexander-akait reviewed Jan 30, 2020

View reviewed changes

alexander-akait reviewed Feb 4, 2020

View reviewed changes

packages/postcss-lowercase-text/src/__tests__/atRules.test.js Show resolved Hide resolved

alexander-akait reviewed Feb 4, 2020

View reviewed changes

packages/postcss-lowercase-text/src/__tests__/atRules.test.js Outdated Show resolved Hide resolved

alexander-akait reviewed Feb 4, 2020

View reviewed changes

anikethsaha force-pushed the feat-postcss-lowercase-text branch from 57998c5 to b1b0698 Compare April 16, 2020 06:53

feat: postcsss-lowercase-text to default preset

af8a940

anikethsaha force-pushed the feat-postcss-lowercase-text branch from b6a1db5 to af8a940 Compare April 27, 2020 05:44

anikethsaha added 2 commits April 27, 2020 12:52

Merge branch 'master' into feat-postcss-lowercase-text

5513454

chore: updated babel/preset-env to 7.9.x

f8d877d

anikethsaha requested a review from alexander-akait April 27, 2020 08:13

alexander-akait reviewed Apr 28, 2020

View reviewed changes

chore: refactoring in lowercase plugin

da03b82

		@@ -1,5 +1,7 @@
		import processCss from './_processCss';

		jest.setTimeout(30000);

Uh oh!

Conversation

anikethsaha commented Jan 16, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov-io commented Jan 16, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

anikethsaha commented Jan 28, 2020

Uh oh!

Uh oh!

Uh oh!

alexander-akait left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

alexander-akait left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

This comment was marked as resolved.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

anikethsaha commented Feb 4, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

alexander-akait commented Mar 24, 2020

Uh oh!

anikethsaha commented Mar 24, 2020

Uh oh!

anikethsaha commented Apr 27, 2020

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

anikethsaha commented Jan 16, 2020 •

edited

Loading

codecov-io commented Jan 16, 2020 •

edited

Loading

anikethsaha commented Feb 4, 2020 •

edited

Loading