From 393a5931646793fea903dbaff81bf5c0e85d1cd6 Mon Sep 17 00:00:00 2001 From: Igor Oleinikov Date: Thu, 8 Jul 2021 17:27:57 -0700 Subject: [PATCH 1/2] Add repro for #233 and accept problematic baselines --- .../minification-only/issue233.ts.baseline | 67 +++++++++++++++++++ .../minification/issue233.ts.baseline | 67 +++++++++++++++++++ .../fixtures/minification/issue233.ts | 17 +++++ 3 files changed, 151 insertions(+) create mode 100644 src/__tests__/baselines/minification-only/issue233.ts.baseline create mode 100644 src/__tests__/baselines/minification/issue233.ts.baseline create mode 100644 src/__tests__/fixtures/minification/issue233.ts diff --git a/src/__tests__/baselines/minification-only/issue233.ts.baseline b/src/__tests__/baselines/minification-only/issue233.ts.baseline new file mode 100644 index 0000000..f897a93 --- /dev/null +++ b/src/__tests__/baselines/minification-only/issue233.ts.baseline @@ -0,0 +1,67 @@ +// Jest Snapshot v1, https://goo.gl/fbAQLP + +exports[`issue233.ts 1`] = ` + +File: issue233.ts +Source code: + + declare const styled: any; + + // repro from #233 + const ValueCalc = styled.div\` + height: calc(var(--line-height) - 5px); + width: calc(100% - 5px); + \`; + + // another case from #233 + styled.div\`--padding-button: calc(var(--padding-button-vertical) - 2px) calc(var(--padding-button-horizontal) - 2px);\` + + // a few more cases + 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))) + \` + + +TypeScript before transform: + + declare const styled: any; + // repro from #233 + const ValueCalc = styled.div \` + height: calc(var(--line-height) - 5px); + width: calc(100% - 5px); + \`; + // another case from #233 + styled.div \`--padding-button: calc(var(--padding-button-vertical) - 2px) calc(var(--padding-button-horizontal) - 2px);\`; + // a few more cases + 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))) + \`; + + +TypeScript after transform: + + declare const styled: any; + // repro from #233 + const ValueCalc = styled.div \`height:calc(var(--line-height)- 5px);width:calc(100% - 5px);\`; + // another case from #233 + styled.div \`--padding-button:calc(var(--padding-button-vertical)- 2px) calc(var(--padding-button-horizontal)- 2px);\`; + // a few more cases + 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)))\`; + + +TypeScript after transpile module: + + // repro from #233 + const ValueCalc = styled.div \`height:calc(var(--line-height)- 5px);width:calc(100% - 5px);\`; + // another case from #233 + styled.div \`--padding-button:calc(var(--padding-button-vertical)- 2px) calc(var(--padding-button-horizontal)- 2px);\`; + // a few more cases + 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)))\`; + + + +`; diff --git a/src/__tests__/baselines/minification/issue233.ts.baseline b/src/__tests__/baselines/minification/issue233.ts.baseline new file mode 100644 index 0000000..720a884 --- /dev/null +++ b/src/__tests__/baselines/minification/issue233.ts.baseline @@ -0,0 +1,67 @@ +// Jest Snapshot v1, https://goo.gl/fbAQLP + +exports[`issue233.ts 1`] = ` + +File: issue233.ts +Source code: + + declare const styled: any; + + // repro from #233 + const ValueCalc = styled.div\` + height: calc(var(--line-height) - 5px); + width: calc(100% - 5px); + \`; + + // another case from #233 + styled.div\`--padding-button: calc(var(--padding-button-vertical) - 2px) calc(var(--padding-button-horizontal) - 2px);\` + + // a few more cases + 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))) + \` + + +TypeScript before transform: + + declare const styled: any; + // repro from #233 + const ValueCalc = styled.div \` + height: calc(var(--line-height) - 5px); + width: calc(100% - 5px); + \`; + // another case from #233 + styled.div \`--padding-button: calc(var(--padding-button-vertical) - 2px) calc(var(--padding-button-horizontal) - 2px);\`; + // a few more cases + 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))) + \`; + + +TypeScript after transform: + + declare const styled: any; + // repro from #233 + const ValueCalc = styled.div.withConfig({ displayName: "ValueCalc" }) \`height:calc(var(--line-height)- 5px);width:calc(100% - 5px);\`; + // another case from #233 + styled.div \`--padding-button:calc(var(--padding-button-vertical)- 2px) calc(var(--padding-button-horizontal)- 2px);\`; + // a few more cases + 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)))\`; + + +TypeScript after transpile module: + + // repro from #233 + const ValueCalc = styled.div.withConfig({ displayName: "ValueCalc" }) \`height:calc(var(--line-height)- 5px);width:calc(100% - 5px);\`; + // another case from #233 + styled.div \`--padding-button:calc(var(--padding-button-vertical)- 2px) calc(var(--padding-button-horizontal)- 2px);\`; + // a few more cases + 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)))\`; + + + +`; diff --git a/src/__tests__/fixtures/minification/issue233.ts b/src/__tests__/fixtures/minification/issue233.ts new file mode 100644 index 0000000..cff8486 --- /dev/null +++ b/src/__tests__/fixtures/minification/issue233.ts @@ -0,0 +1,17 @@ +declare const styled: any; + +// repro from #233 +const ValueCalc = styled.div` + height: calc(var(--line-height) - 5px); + width: calc(100% - 5px); +`; + +// another case from #233 +styled.div`--padding-button: calc(var(--padding-button-vertical) - 2px) calc(var(--padding-button-horizontal) - 2px);` + +// a few more cases +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))) +` From 915b202b143924f3cebf87f93b5d7020279fa70a Mon Sep 17 00:00:00 2001 From: Igor Oleinikov Date: Thu, 8 Jul 2021 17:29:11 -0700 Subject: [PATCH 2/2] Fix #233 by counting braces in minifying state machine --- .../minification-only/issue233.ts.baseline | 12 ++--- .../minification/issue233.ts.baseline | 12 ++--- src/minify.ts | 45 ++++++++++++++----- 3 files changed, 46 insertions(+), 23 deletions(-) diff --git a/src/__tests__/baselines/minification-only/issue233.ts.baseline b/src/__tests__/baselines/minification-only/issue233.ts.baseline index f897a93..332c453 100644 --- a/src/__tests__/baselines/minification-only/issue233.ts.baseline +++ b/src/__tests__/baselines/minification-only/issue233.ts.baseline @@ -46,21 +46,21 @@ TypeScript after transform: declare const styled: any; // repro from #233 - const ValueCalc = styled.div \`height:calc(var(--line-height)- 5px);width:calc(100% - 5px);\`; + const ValueCalc = styled.div \`height:calc(var(--line-height) - 5px);width:calc(100% - 5px);\`; // another case from #233 - styled.div \`--padding-button:calc(var(--padding-button-vertical)- 2px) calc(var(--padding-button-horizontal)- 2px);\`; + styled.div \`--padding-button:calc(var(--padding-button-vertical) - 2px)calc(var(--padding-button-horizontal) - 2px);\`; // a few more cases - 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)))\`; + 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)))\`; TypeScript after transpile module: // repro from #233 - const ValueCalc = styled.div \`height:calc(var(--line-height)- 5px);width:calc(100% - 5px);\`; + const ValueCalc = styled.div \`height:calc(var(--line-height) - 5px);width:calc(100% - 5px);\`; // another case from #233 - styled.div \`--padding-button:calc(var(--padding-button-vertical)- 2px) calc(var(--padding-button-horizontal)- 2px);\`; + styled.div \`--padding-button:calc(var(--padding-button-vertical) - 2px)calc(var(--padding-button-horizontal) - 2px);\`; // a few more cases - 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)))\`; + 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)))\`; diff --git a/src/__tests__/baselines/minification/issue233.ts.baseline b/src/__tests__/baselines/minification/issue233.ts.baseline index 720a884..191f63c 100644 --- a/src/__tests__/baselines/minification/issue233.ts.baseline +++ b/src/__tests__/baselines/minification/issue233.ts.baseline @@ -46,21 +46,21 @@ TypeScript after transform: declare const styled: any; // repro from #233 - const ValueCalc = styled.div.withConfig({ displayName: "ValueCalc" }) \`height:calc(var(--line-height)- 5px);width:calc(100% - 5px);\`; + const ValueCalc = styled.div.withConfig({ displayName: "ValueCalc" }) \`height:calc(var(--line-height) - 5px);width:calc(100% - 5px);\`; // another case from #233 - styled.div \`--padding-button:calc(var(--padding-button-vertical)- 2px) calc(var(--padding-button-horizontal)- 2px);\`; + styled.div \`--padding-button:calc(var(--padding-button-vertical) - 2px)calc(var(--padding-button-horizontal) - 2px);\`; // a few more cases - 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)))\`; + 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)))\`; TypeScript after transpile module: // repro from #233 - const ValueCalc = styled.div.withConfig({ displayName: "ValueCalc" }) \`height:calc(var(--line-height)- 5px);width:calc(100% - 5px);\`; + const ValueCalc = styled.div.withConfig({ displayName: "ValueCalc" }) \`height:calc(var(--line-height) - 5px);width:calc(100% - 5px);\`; // another case from #233 - styled.div \`--padding-button:calc(var(--padding-button-vertical)- 2px) calc(var(--padding-button-horizontal)- 2px);\`; + styled.div \`--padding-button:calc(var(--padding-button-vertical) - 2px)calc(var(--padding-button-horizontal) - 2px);\`; // a few more cases - 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)))\`; + 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)))\`; diff --git a/src/minify.ts b/src/minify.ts index 4faab8c..25f1cfb 100644 --- a/src/minify.ts +++ b/src/minify.ts @@ -2,10 +2,15 @@ import * as ts from 'typescript'; import { isNoSubstitutionTemplateLiteral, isTemplateExpression } from './ts-is-kind'; type State = ';' | ';$' | 'x' | ' ' | '\n' | '"' | '(' | '\'' | '/' | '//' | ';/' | ';//' | '/$' | '//$' | '/*' | '/**' | ';/*' | ';/**' | '/*$' | '/*$*'; -type ReducerResult = { emit?: string; skipEmit?: boolean; state?: State; } | void; +type StateDataDef = { + ['(']: { count: number } +} +type StateData = K extends keyof StateDataDef ? StateDataDef[K] : void +type StateResult = { [K in State]: K extends keyof StateDataDef ? [K, StateDataDef[K]] : K }[State] +type ReducerResult = { emit?: string; skipEmit?: boolean; state?: StateResult; } | void; type StateMachine = { [K in State]: { - next?(ch: string): ReducerResult; + next?(ch: string, data: StateData): ReducerResult; flush?(last: boolean): ReducerResult; } }; @@ -21,7 +26,8 @@ function isSpace(ch: string) { const stateMachine: StateMachine = { ';': { next(ch) { - if (ch == '\'' || ch == '"' || ch == '(') return { state: ch } + if (ch == '(') return { state: ['(', {count: 1}]} + if (ch == '\'' || ch == '"') return { state: ch } if (isSpace(ch)) return { skipEmit: true } if (ch == '/') return { state: ';/', skipEmit: true } if (isSymbol(ch)) return; @@ -33,7 +39,8 @@ const stateMachine: StateMachine = { }, ';$': { // after placeholder next(ch) { - if (ch == '\'' || ch == '"' || ch == '(') return { state: ch } + if (ch == '(') return { state: ['(', {count: 1}]} + if (ch == '\'' || ch == '"') return { state: ch } if (isSpace(ch)) return { skipEmit: true, state: ' ' } // we may need a space if (ch == '/') return { state: '/', skipEmit: true } if (isSymbol(ch)) return { state: ';' }; @@ -42,7 +49,8 @@ const stateMachine: StateMachine = { }, 'x': { next(ch) { - if (ch == '\'' || ch == '"' || ch == '(') return { state: ch } + if (ch == '(') return { state: ['(', {count: 1}]} + if (ch == '\'' || ch == '"') return { state: ch } if (isSpace(ch)) return { state: ' ', skipEmit: true } if (ch == '/') return { state: '/', skipEmit: true } if (isSymbol(ch)) return { state: ';' }; @@ -50,7 +58,8 @@ const stateMachine: StateMachine = { }, ' ': { // may need space next(ch) { - if (ch == '\'' || ch == '"' || ch == '(') return { state: ch, emit: ' ' + ch } + if (ch == '(') return { state: ['(', {count: 1}], emit: ' ' + ch} + if (ch == '\'' || ch == '"') return { state: ch, emit: ' ' + ch } if (isSpace(ch)) return { state: ' ', skipEmit: true } if (ch == '/') return { state: '/', skipEmit: true } if (isSymbol(ch)) return { state: ';' }; @@ -62,7 +71,8 @@ const stateMachine: StateMachine = { }, '\n': { // may need new line next(ch) { - if (ch == '\'' || ch == '"' || ch == '(') return { state: ch, emit: '\n' + ch } + if (ch == '(') return { state: ['(', {count: 1}], emit: '\n' + ch} + if (ch == '\'' || ch == '"') return { state: ch, emit: '\n' + ch } if (isSpace(ch)) return { state: '\n', skipEmit: true } if (ch == '/') return { state: '/', emit: '\n' } if (isSymbol(ch)) return { state: ';', emit: '\n' + ch }; @@ -80,8 +90,13 @@ const stateMachine: StateMachine = { } }, '(': { - next(ch) { - if (ch == ')') return { state: ';' }; // maybe return ' '? then it'd always add space after + next(ch, { count }) { + if (ch == '(') return { state: ['(', { count: count+1 }] } + if (ch == ')') + if (count > 1) + return { state: ['(', { count: count-1 }] } + else + return { state: ';' }; // maybe return ' '? then it'd always add space after } }, '/': { @@ -179,6 +194,7 @@ const stateMachine: StateMachine = { export function createMinifier(): (next: string, last?: boolean) => string { let state: State = ';'; + let stateData: StateData = undefined as StateData<';'> return (next, last = false) => { let minified = ''; @@ -189,7 +205,14 @@ export function createMinifier(): (next: string, last?: boolean) => string { minified += ch; } else { if (result.state !== undefined) - state = result.state; + { + if (typeof result.state === 'string') + state = result.state; + else { + state = result.state[0] + stateData = result.state[1] + } + } if (result.emit !== undefined) minified += result.emit; else if (result.skipEmit !== true && ch !== undefined) @@ -203,7 +226,7 @@ export function createMinifier(): (next: string, last?: boolean) => string { const ch = next[pos++]; const reducer = stateMachine[state]; const prevState = state; - const reducerResult = reducer.next && reducer.next(ch); + const reducerResult = reducer.next && reducer.next(ch, stateData as any); apply(reducerResult, ch) // console.log('next(', { ch, state: prevState }, '): ', reducerResult, ' -> ', { state, minified }); }