Skip to content

Commit 1e76ace

Browse files
IgorbekIgorbek
authored
Merge pull request #436 from Igorbek/fix233
Fix incorrect minification of calc()
2 parents bf7679c + 5d31899 commit 1e76ace

File tree

4 files changed

+185
-11
lines changed

4 files changed

+185
-11
lines changed

‎src/__tests__/baselines/minification-only/issue233.ts.baseline

+67
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,67 @@
1+
// Jest Snapshot v1, https://goo.gl/fbAQLP
2+
3+
exports[`issue233.ts 1`] = `
4+
5+
File: issue233.ts
6+
Source code:
7+
8+
declare const styled: any;
9+
10+
// repro from #233
11+
const ValueCalc = styled.div\`
12+
height: calc(var(--line-height) - 5px);
13+
width: calc(100% - 5px);
14+
\`;
15+
16+
// another case from #233
17+
styled.div\`--padding-button: calc(var(--padding-button-vertical) - 2px) calc(var(--padding-button-horizontal) - 2px);\`
18+
19+
// a few more cases
20+
styled.div\`
21+
width: calc( 1%)
22+
width: calc( 1% + var(--a) - calc(2%))
23+
width: calc( 1% + var(--a) - calc(2%) + calc( 1px + calc(1px + 2px) + var(--a)))
24+
\`
25+
26+
27+
TypeScript before transform:
28+
29+
declare const styled: any;
30+
// repro from #233
31+
const ValueCalc = styled.div \`
32+
height: calc(var(--line-height) - 5px);
33+
width: calc(100% - 5px);
34+
\`;
35+
// another case from #233
36+
styled.div \`--padding-button: calc(var(--padding-button-vertical) - 2px) calc(var(--padding-button-horizontal) - 2px);\`;
37+
// a few more cases
38+
styled.div \`
39+
width: calc( 1%)
40+
width: calc( 1% + var(--a) - calc(2%))
41+
width: calc( 1% + var(--a) - calc(2%) + calc( 1px + calc(1px + 2px) + var(--a)))
42+
\`;
43+
44+
45+
TypeScript after transform:
46+
47+
declare const styled: any;
48+
// repro from #233
49+
const ValueCalc = styled.div \`height:calc(var(--line-height) - 5px);width:calc(100% - 5px);\`;
50+
// another case from #233
51+
styled.div \`--padding-button:calc(var(--padding-button-vertical) - 2px)calc(var(--padding-button-horizontal) - 2px);\`;
52+
// a few more cases
53+
styled.div \`width:calc( 1%)width:calc( 1% + var(--a) - calc(2%))width:calc( 1% + var(--a) - calc(2%) + calc( 1px + calc(1px + 2px) + var(--a)))\`;
54+
55+
56+
TypeScript after transpile module:
57+
58+
// repro from #233
59+
const ValueCalc = styled.div \`height:calc(var(--line-height) - 5px);width:calc(100% - 5px);\`;
60+
// another case from #233
61+
styled.div \`--padding-button:calc(var(--padding-button-vertical) - 2px)calc(var(--padding-button-horizontal) - 2px);\`;
62+
// a few more cases
63+
styled.div \`width:calc( 1%)width:calc( 1% + var(--a) - calc(2%))width:calc( 1% + var(--a) - calc(2%) + calc( 1px + calc(1px + 2px) + var(--a)))\`;
64+
65+
66+
67+
`;

‎src/__tests__/baselines/minification/issue233.ts.baseline

+67
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,67 @@
1+
// Jest Snapshot v1, https://goo.gl/fbAQLP
2+
3+
exports[`issue233.ts 1`] = `
4+
5+
File: issue233.ts
6+
Source code:
7+
8+
declare const styled: any;
9+
10+
// repro from #233
11+
const ValueCalc = styled.div\`
12+
height: calc(var(--line-height) - 5px);
13+
width: calc(100% - 5px);
14+
\`;
15+
16+
// another case from #233
17+
styled.div\`--padding-button: calc(var(--padding-button-vertical) - 2px) calc(var(--padding-button-horizontal) - 2px);\`
18+
19+
// a few more cases
20+
styled.div\`
21+
width: calc( 1%)
22+
width: calc( 1% + var(--a) - calc(2%))
23+
width: calc( 1% + var(--a) - calc(2%) + calc( 1px + calc(1px + 2px) + var(--a)))
24+
\`
25+
26+
27+
TypeScript before transform:
28+
29+
declare const styled: any;
30+
// repro from #233
31+
const ValueCalc = styled.div \`
32+
height: calc(var(--line-height) - 5px);
33+
width: calc(100% - 5px);
34+
\`;
35+
// another case from #233
36+
styled.div \`--padding-button: calc(var(--padding-button-vertical) - 2px) calc(var(--padding-button-horizontal) - 2px);\`;
37+
// a few more cases
38+
styled.div \`
39+
width: calc( 1%)
40+
width: calc( 1% + var(--a) - calc(2%))
41+
width: calc( 1% + var(--a) - calc(2%) + calc( 1px + calc(1px + 2px) + var(--a)))
42+
\`;
43+
44+
45+
TypeScript after transform:
46+
47+
declare const styled: any;
48+
// repro from #233
49+
const ValueCalc = styled.div.withConfig({ displayName: "ValueCalc" }) \`height:calc(var(--line-height) - 5px);width:calc(100% - 5px);\`;
50+
// another case from #233
51+
styled.div \`--padding-button:calc(var(--padding-button-vertical) - 2px)calc(var(--padding-button-horizontal) - 2px);\`;
52+
// a few more cases
53+
styled.div \`width:calc( 1%)width:calc( 1% + var(--a) - calc(2%))width:calc( 1% + var(--a) - calc(2%) + calc( 1px + calc(1px + 2px) + var(--a)))\`;
54+
55+
56+
TypeScript after transpile module:
57+
58+
// repro from #233
59+
const ValueCalc = styled.div.withConfig({ displayName: "ValueCalc" }) \`height:calc(var(--line-height) - 5px);width:calc(100% - 5px);\`;
60+
// another case from #233
61+
styled.div \`--padding-button:calc(var(--padding-button-vertical) - 2px)calc(var(--padding-button-horizontal) - 2px);\`;
62+
// a few more cases
63+
styled.div \`width:calc( 1%)width:calc( 1% + var(--a) - calc(2%))width:calc( 1% + var(--a) - calc(2%) + calc( 1px + calc(1px + 2px) + var(--a)))\`;
64+
65+
66+
67+
`;

‎src/__tests__/fixtures/minification/issue233.ts

+17
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
declare const styled: any;
2+
3+
// repro from #233
4+
const ValueCalc = styled.div`
5+
height: calc(var(--line-height) - 5px);
6+
width: calc(100% - 5px);
7+
`;
8+
9+
// another case from #233
10+
styled.div`--padding-button: calc(var(--padding-button-vertical) - 2px) calc(var(--padding-button-horizontal) - 2px);`
11+
12+
// a few more cases
13+
styled.div`
14+
width: calc( 1%)
15+
width: calc( 1% + var(--a) - calc(2%))
16+
width: calc( 1% + var(--a) - calc(2%) + calc( 1px + calc(1px + 2px) + var(--a)))
17+
`

‎src/minify.ts

+34-11
Original file line numberDiff line numberDiff line change
@@ -2,10 +2,15 @@ import * as ts from 'typescript';
22
import { isNoSubstitutionTemplateLiteral, isTemplateExpression } from './ts-is-kind';
33

44
type State = ';' | ';$' | 'x' | ' ' | '\n' | '"' | '(' | '\'' | '/' | '//' | ';/' | ';//' | '/$' | '//$' | '/*' | '/**' | ';/*' | ';/**' | '/*$' | '/*$*';
5-
type ReducerResult = { emit?: string; skipEmit?: boolean; state?: State; } | void;
5+
type StateDataDef = {
6+
['(']: { count: number }
7+
}
8+
type StateData<K extends State> = K extends keyof StateDataDef ? StateDataDef[K] : void
9+
type StateResult = { [K in State]: K extends keyof StateDataDef ? [K, StateDataDef[K]] : K }[State]
10+
type ReducerResult = { emit?: string; skipEmit?: boolean; state?: StateResult; } | void;
611
type StateMachine = {
712
[K in State]: {
8-
next?(ch: string): ReducerResult;
13+
next?(ch: string, data: StateData<K>): ReducerResult;
914
flush?(last: boolean): ReducerResult;
1015
}
1116
};
@@ -21,7 +26,8 @@ function isSpace(ch: string) {
2126
const stateMachine: StateMachine = {
2227
';': {
2328
next(ch) {
24-
if (ch == '\'' || ch == '"' || ch == '(') return { state: ch }
29+
if (ch == '(') return { state: ['(', {count: 1}]}
30+
if (ch == '\'' || ch == '"') return { state: ch }
2531
if (isSpace(ch)) return { skipEmit: true }
2632
if (ch == '/') return { state: ';/', skipEmit: true }
2733
if (isSymbol(ch)) return;
@@ -33,7 +39,8 @@ const stateMachine: StateMachine = {
3339
},
3440
';$': { // after placeholder
3541
next(ch) {
36-
if (ch == '\'' || ch == '"' || ch == '(') return { state: ch }
42+
if (ch == '(') return { state: ['(', {count: 1}]}
43+
if (ch == '\'' || ch == '"') return { state: ch }
3744
if (isSpace(ch)) return { skipEmit: true, state: ' ' } // we may need a space
3845
if (ch == '/') return { state: '/', skipEmit: true }
3946
if (isSymbol(ch)) return { state: ';' };
@@ -42,15 +49,17 @@ const stateMachine: StateMachine = {
4249
},
4350
'x': {
4451
next(ch) {
45-
if (ch == '\'' || ch == '"' || ch == '(') return { state: ch }
52+
if (ch == '(') return { state: ['(', {count: 1}]}
53+
if (ch == '\'' || ch == '"') return { state: ch }
4654
if (isSpace(ch)) return { state: ' ', skipEmit: true }
4755
if (ch == '/') return { state: '/', skipEmit: true }
4856
if (isSymbol(ch)) return { state: ';' };
4957
}
5058
},
5159
' ': { // may need space
5260
next(ch) {
53-
if (ch == '\'' || ch == '"' || ch == '(') return { state: ch, emit: ' ' + ch }
61+
if (ch == '(') return { state: ['(', {count: 1}], emit: ' ' + ch}
62+
if (ch == '\'' || ch == '"') return { state: ch, emit: ' ' + ch }
5463
if (isSpace(ch)) return { state: ' ', skipEmit: true }
5564
if (ch == '/') return { state: '/', skipEmit: true }
5665
if (isSymbol(ch)) return { state: ';' };
@@ -62,7 +71,8 @@ const stateMachine: StateMachine = {
6271
},
6372
'\n': { // may need new line
6473
next(ch) {
65-
if (ch == '\'' || ch == '"' || ch == '(') return { state: ch, emit: '\n' + ch }
74+
if (ch == '(') return { state: ['(', {count: 1}], emit: '\n' + ch}
75+
if (ch == '\'' || ch == '"') return { state: ch, emit: '\n' + ch }
6676
if (isSpace(ch)) return { state: '\n', skipEmit: true }
6777
if (ch == '/') return { state: '/', emit: '\n' }
6878
if (isSymbol(ch)) return { state: ';', emit: '\n' + ch };
@@ -80,8 +90,13 @@ const stateMachine: StateMachine = {
8090
}
8191
},
8292
'(': {
83-
next(ch) {
84-
if (ch == ')') return { state: ';' }; // maybe return ' '? then it'd always add space after
93+
next(ch, { count }) {
94+
if (ch == '(') return { state: ['(', { count: count+1 }] }
95+
if (ch == ')')
96+
if (count > 1)
97+
return { state: ['(', { count: count-1 }] }
98+
else
99+
return { state: ';' }; // maybe return ' '? then it'd always add space after
85100
}
86101
},
87102
'/': {
@@ -179,6 +194,7 @@ const stateMachine: StateMachine = {
179194

180195
export function createMinifier(): (next: string, last?: boolean) => string {
181196
let state: State = ';';
197+
let stateData: StateData<State> = undefined as StateData<';'>
182198

183199
return (next, last = false) => {
184200
let minified = '';
@@ -189,7 +205,14 @@ export function createMinifier(): (next: string, last?: boolean) => string {
189205
minified += ch;
190206
} else {
191207
if (result.state !== undefined)
192-
state = result.state;
208+
{
209+
if (typeof result.state === 'string')
210+
state = result.state;
211+
else {
212+
state = result.state[0]
213+
stateData = result.state[1]
214+
}
215+
}
193216
if (result.emit !== undefined)
194217
minified += result.emit;
195218
else if (result.skipEmit !== true && ch !== undefined)
@@ -203,7 +226,7 @@ export function createMinifier(): (next: string, last?: boolean) => string {
203226
const ch = next[pos++];
204227
const reducer = stateMachine[state];
205228
const prevState = state;
206-
const reducerResult = reducer.next && reducer.next(ch);
229+
const reducerResult = reducer.next && reducer.next(ch, stateData as any);
207230
apply(reducerResult, ch)
208231
// console.log('next(', { ch, state: prevState }, '): ', reducerResult, ' -> ', { state, minified });
209232
}

0 commit comments

Comments
 (0)