Minify classnames #183
Minify classnames #183
Conversation
| @@ -76,4 +87,18 @@ describe('preval-extract/prevalStyles', () => { | |||
| "css.named('header', 'filename.js')`color: #ffffff`" | |||
| ); | |||
| }); | |||
|
|
|||
| it('test', () => { | |||
There was a problem hiding this comment.
nit: it('minifies class names',
There was a problem hiding this comment.
Sorry about that, I've noticed it just now 🙏
| } | ||
| ); | ||
|
|
||
| expect(className.startsWith('ln')).toBeTruthy(); |
There was a problem hiding this comment.
You could also assert on the digits count
|
|
||
| import getReplacement from './getReplacement'; | ||
| import { | ||
| instantiateModule, | ||
| clearLocalModulesFromCache, | ||
| } from '../lib/moduleSystem'; | ||
|
|
||
| function getMinifiedClassName(className: string) { | ||
| return `ln${slugify(className).substr(0, 6)}`; |
There was a problem hiding this comment.
I'm afraid this may lead to clashing hashes
There was a problem hiding this comment.
Me too, but I'm not capable to calculate in which case. Should I introduce some algorithm which targets only 6 chars long string to avoid substr?
There was a problem hiding this comment.
I think it's safer to stick with our 7-char hash, at least we stay consistent with dev vs prod.
There was a problem hiding this comment.
@thymikee I'm lame, I don't get it where I got this code. Slugify return 6 chars hash implicitly. Substr is not needed so I assume everything will be OK. 🤷♀️
| ); | ||
|
|
||
| expect(/ln[\w]{6}/.test(className)).toBeTruthy(); | ||
| expect(className.startsWith('ln')).toBeTruthy(); |
There was a problem hiding this comment.
I think this assertion is unnecessary when the previous one already tests this.
|
|
||
| import getReplacement from './getReplacement'; | ||
| import { | ||
| instantiateModule, | ||
| clearLocalModulesFromCache, | ||
| } from '../lib/moduleSystem'; | ||
|
|
||
| function getMinifiedClassName(className: string) { | ||
| return `ln${slugify(className)}`; |
There was a problem hiding this comment.
Not really sure if slugifying an already slugified string is a good idea. I'd prefer to change https://github.com/callstack/linaria/blob/master/src/babel/preval-extract/index.js#L111 to set the title to ln and work from there.
There was a problem hiding this comment.
Just replace slugify with short-hash
| } | ||
| ); | ||
|
|
||
| expect(/ln[\w]{6}/.test(className)).toBeTruthy(); |
There was a problem hiding this comment.
This regex should be [a-zA-Z0-9]
There was a problem hiding this comment.
You are right, I forget that \w match also _. 👶
|
@jukben please don't run prettier over package.json |
jukben
force-pushed
the
feat/minify-classnames
branch
from
cc5eb8b to
9dd2c52
Compare
|
It looks like I have to turn off prettier for Linaria package, sorry about that. 😔 |
This PR added option to minify class names. Closes #175
Added corresponding test.
You can use it like
will produce classs names within pattern
/ln[a-z0-9]{6}/