Skip to content

Commit

Permalink
fix(styling): Fix parent selectors in compat mode (#2743)
Browse files Browse the repository at this point in the history
PR #2741 added the ability to target parent modifier class names to build selectors, but doesn't work in compat mode because compat mode merges all the CSS class names into a single, new class name. I added a `parentModifiers` function that handles this for both React Kit and CSS Kit and updated stencils to add inert class names for the purpose of always matching a selector.

I also fixed a bug where passing `undefined` to a Stencil for a modifier key resulted in overriding `defaultModifiers`.

[category:Styling]
  • Loading branch information
NicholasBoll committed May 16, 2024
1 parent 2313244 commit 78514b9
Show file tree
Hide file tree
Showing 7 changed files with 235 additions and 18 deletions.
3 changes: 2 additions & 1 deletion modules/styling-transform/lib/styleTransform.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import {handleCssVar} from './utils/handleCssVar';
import {Config, NodeTransformer, ObjectTransform, TransformerContext} from './utils/types';
import {handleKeyframes} from './utils/handleKeyframes';
import {handleInjectGlobal} from './utils/handleInjectGlobal';
import {handleParentModifier} from './utils/handleParentModifier';

export type NestedStyleObject = {[key: string]: string | NestedStyleObject};

Expand Down Expand Up @@ -140,7 +141,7 @@ export function withDefaultContext(
objectTransforms: [] as ObjectTransform[],
transform: handleTransformers(transformers || defaultTransformers),
...input,
propertyTransforms: [handleCalc, handlePx2Rem, handleCssVar].concat(
propertyTransforms: [handleCalc, handlePx2Rem, handleCssVar, handleParentModifier].concat(
input.propertyTransforms || []
),
} as TransformerContext;
Expand Down
6 changes: 3 additions & 3 deletions modules/styling-transform/lib/utils/handleCreateStencil.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import {getHash} from './getHash';
* Handle all arguments of the CallExpression `createStencil()`
*/
export const handleCreateStencil: NodeTransformer = (node, context) => {
const {checker, prefix, names, extractedNames} = context;
const {checker, names, extractedNames} = context;
/**
* This will match whenever a `createStencil()` call expression is encountered. It will loop
* over all the config to extract variables and styles.
Expand Down Expand Up @@ -423,11 +423,11 @@ function createStyleReplacementNode(
className: string,
context: TransformerContext
) {
const {prefix, names, extractedNames} = context;
const {names, extractedNames} = context;
const serialized = serializeStyles(node, styleObj, `.${className}{%s}`, context);

const varName = getVarName(node);
const value = `${prefix}-${serialized.name}`;
const value = `css-${serialized.name}`;
names[varName] = value;
extractedNames[value] = getClassName(varName, context);

Expand Down
29 changes: 29 additions & 0 deletions modules/styling-transform/lib/utils/handleParentModifier.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
import ts from 'typescript';

import {parentModifier} from '@workday/canvas-kit-styling';
import {createPropertyTransform} from '../createPropertyTransform';
import {parseNodeToStaticValue} from './parseNodeToStaticValue';

export const handleParentModifier = createPropertyTransform((node, context) => {
const {names, extractedNames} = context;

if (
ts.isComputedPropertyName(node) &&
ts.isCallExpression(node.expression) &&
ts.isIdentifier(node.expression.expression) &&
node.expression.expression.text === 'parentModifier'
) {
const args = node.expression.arguments.map(arg => parseNodeToStaticValue(arg, context));
const hash = args[0].toString().replace('css-', '');

// add a mapping from `css-{hash}` to `{hash}` for extraction string replacement
names[args[0]] = hash;

// map the `{hash}` to the extracted CSS class name
extractedNames[hash] = extractedNames[args[0]];

return parentModifier(args[0].toString());
}

return;
});
37 changes: 35 additions & 2 deletions modules/styling-transform/spec/utils/handleCreateStencil.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -355,7 +355,7 @@ describe('handleCreateStencil', () => {

it('should handle parsing modifiers with ObjectLiteralExpressions', () => {
const program = createProgramFromSource(`
import {createStencil} from '@workday/canvas-kit-styling';
import {createStencil, parentModifier} from '@workday/canvas-kit-styling';
const buttonStencil = createStencil({
vars: {
Expand All @@ -369,12 +369,45 @@ describe('handleCreateStencil', () => {
}
}
})
const childStencil = createStencil({
base: {
[parentModifier(buttonStencil.modifiers.size.large)]: {
color: 'blue',
}
}
})
`);

const result = transform(program, 'test.ts', withDefaultContext(program.getTypeChecker()));
const styles = {};
const names = {};
const extractedNames = {};

const result = transform(
program,
'test.ts',
withDefaultContext(program.getTypeChecker(), {styles, names, extractedNames})
);

expect(result).toMatch(/large: { name: "[0-9a-z]+", styles: "padding:20px;" }/);
expect(result).toMatch(/small: { name: "[0-9a-z]+", styles: "padding:10px;" }/);

// runtime selector
expect(result).toContain(
`.${names['buttonStencil.modifiers.size.large'].replace('css-', '')} :where(&){color:blue;}`
);

// extracted selector
expect(getFile(styles, 'test.css')).toContainEqual(
compileCSS(`
.css-child {
box-sizing: border-box;
}
.css-button--size-large :where(.css-child) {
color: blue;
}
`)
);
});

it('should handle parsing modifiers with ObjectLiteralExpressions', () => {
Expand Down
82 changes: 82 additions & 0 deletions modules/styling-transform/spec/utils/handleParentModifier.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,82 @@
import ts from 'typescript';

import {createProgramFromSource} from '../createProgramFromSource';

import {transform, withDefaultContext, _reset} from '../../lib/styleTransform';
import {compileCSS} from '../../lib/utils/createStyleObjectNode';

describe('handleParentModifier', () => {
let program: ts.Program;
let result: string;

const styles = {};
const names = {};
const extractedNames = {};

beforeAll(() => {
_reset();
program = createProgramFromSource(`
import {createStencil, parentModifier} from '@workday/canvas-kit-styling';
const buttonStencil = createStencil({
vars: {
color: 'red'
},
base: {},
modifiers: {
size: {
large: {padding: 20},
small: {padding: 10}
}
}
})
const childStencil = createStencil({
base: {
[parentModifier(buttonStencil.modifiers.size.large)]: {
color: 'blue',
}
}
})
`);

result = transform(
program,
'test.ts',
withDefaultContext(program.getTypeChecker(), {styles, names, extractedNames})
);
});

it('should add a mapping from the CSS class name to the hash to the names cache', () => {
expect(names).toHaveProperty(
names['buttonStencil.modifiers.size.large'],
names['buttonStencil.modifiers.size.large'].replace('css-', '')
);
});

it('should add a mapping from the hash to the extracted CSS class name to the extractedNames cache', () => {
expect(extractedNames).toHaveProperty(
names['buttonStencil.modifiers.size.large'].replace('css-', ''),
'css-button--size-large'
);
});

it('should transform the runtime to include a selector with only the hash', () => {
expect(result).toContain(
`.${names['buttonStencil.modifiers.size.large'].replace('css-', '')} :where(&){color:blue;}`
);
});

it('should extract CSS to include the fully qualified modifier name', () => {
expect(styles['test.css']).toContainEqual(
compileCSS(`
.css-child {
box-sizing: border-box;
}
.css-button--size-large :where(.css-child) {
color: blue;
}
`)
);
});
});
45 changes: 42 additions & 3 deletions modules/styling/lib/cs.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1049,6 +1049,26 @@ function combineClassNames(input: (string | undefined)[]): string {
.join(' ');
}

/**
* This function is used in conjunction with Stencils to get a selector to match the current element
* and a parent modifier. The selector will match a parent element with a modifier applied and the current
* element. It should be used on the `base` config of a Stencil.
*
* ```ts
* const childStencil = createStencil({
* base: {
* // base styles
* [parentModifier(parentStencil.modifiers.size.large)]: {
* // maybe adjust padding of this element
* }
* }
* })
* ```
*/
export function parentModifier(value: string) {
return `.${value.replace('css-', '')} :where(&)`;
}

/**
* Creates a reuseable Stencil for styling elements. It takes vars, base styles, modifiers, and
* compound modifiers.
Expand Down Expand Up @@ -1087,6 +1107,7 @@ export function createStencil<
result[modifierKey] = createStyles(
typeof modifier === 'function' ? modifier(_vars) : modifier
);

return result;
}, {});

Expand Down Expand Up @@ -1127,14 +1148,32 @@ export function createStencil<
)
: () => '';

const stencil: Stencil<M, V, E, ID> = ((input: {}) => {
const inputModifiers = {...composes?.defaultModifiers, ...defaultModifiers, ...input};
const stencil: Stencil<M, V, E, ID> = ((input: Record<string, string>) => {
const inputModifiers = {...composes?.defaultModifiers, ...defaultModifiers};
// Only override defaults if a value is defined
for (const key in input) {
if (input[key]) {
// @ts-ignore
inputModifiers[key] = input[key];
}
}
const composesReturn = composes?.(inputModifiers as any);
const modifierClasses = _modifiers(inputModifiers);
return {
className: combineClassNames([
composesReturn?.className,
_base,
_modifiers(inputModifiers),
modifierClasses,
// For compat mode, we need to add inert class names of modifiers where the `css-` prefix is
// removed. For example, `css-base css-mod-1` will become `css-base css-mod-1 mod-1`. The
// modifier class without the prefix will be ignored by the Emotion CSS `cx` function and
// will remain for the `parentModifier` function to still select on. We decided to add these
// inert class names instead of adding `data-m-*` attributes because the output DOM looks
// much cleaner and it saves bytes on the bundled output.
modifierClasses
.split(' ')
.map(c => c.replace('css-', ''))
.join(' '),
compound ? _compound(inputModifiers) : '',
]),
style: {...composesReturn?.style, ..._vars(input || {})},
Expand Down
51 changes: 42 additions & 9 deletions modules/styling/spec/cs.spec.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -529,7 +529,9 @@ describe('cs', () => {

expect(screen.getByTestId('test1')).toHaveStyle({padding: '10px'});
expect(screen.getByTestId('test2')).toHaveStyle({padding: '20px'});
expect(myStencil({size: 'large'}).className.split(' ')).toHaveLength(2);

expect(myStencil({size: 'large'}).className).toContain(myStencil.base);
expect(myStencil({size: 'large'}).className).toContain(myStencil.modifiers.size.large);

// type signature of stencil call expression
type Arg = Parameters<typeof myStencil>[0];
Expand Down Expand Up @@ -577,11 +579,30 @@ describe('cs', () => {
},
});

expect(myStencil({position: 'start', size: 'large'}).className).toEqual(
expect(myStencil({position: 'start', size: 'large'}).className).toContain(
`${myStencil.base} ${myStencil.modifiers.size.large} ${myStencil.modifiers.position.start}`
);
});

it('should add the hash of the modifier to the className to handle parentModifier selectors in compat mode', () => {
const myStencil = createStencil({
base: {},
modifiers: {
size: {
large: {},
},
},
});

const {className} = myStencil({size: 'large'});

expect(className).toEqual(
`${myStencil.base} ${
myStencil.modifiers.size.large
} ${myStencil.modifiers.size.large.replace('css-', '')}`
);
});

it('should default modifiers if no modifier override is passed', () => {
const myStencil = createStencil({
base: {
Expand Down Expand Up @@ -653,7 +674,13 @@ describe('cs', () => {
});

// 4 class names - base, size, position, size-position
expect(myStencil({size: 'large', position: 'start'}).className.split(' ')).toHaveLength(4);
expect(myStencil({size: 'large', position: 'start'}).className.split(' ')).toHaveLength(6); // 4 + 2 modifier hashes
const {className} = myStencil({size: 'large', position: 'start'});

expect(className).toContain(myStencil.base);
expect(className).toContain(myStencil.modifiers.size.large);
expect(className).toContain(myStencil.modifiers.position.start);
// we don't have access to the compound modifier class name
});

it('should return access to variables for use in other components', () => {
Expand Down Expand Up @@ -796,9 +823,6 @@ describe('cs', () => {
width: '0',
},
},
foo: {
true: {},
},
},
});

Expand All @@ -818,7 +842,7 @@ describe('cs', () => {
expect(result2.style).toHaveProperty(myStencil.vars.width, 'zero');

// match base and width modifier
expect(result2.className).toEqual(`${myStencil.base} ${myStencil.modifiers.width.zero}`);
expect(result2.className).toContain(`${myStencil.base} ${myStencil.modifiers.width.zero}`);
});

it('should convert "true" modifiers into boolean', () => {
Expand Down Expand Up @@ -959,7 +983,13 @@ describe('cs', () => {

const {className} = extendedStencil({size: 'large'});

expect(className.split(' ')).toHaveProperty('length', 5);
expect(className.split(' ')).toHaveProperty('length', 6); // 1 base + 1 extended + 1 size modifier + 2 compound modifiers + 1 modifier hash

expect(className).toContain(baseStencil.base);
extendedStencil.base.split(' ').forEach(c => {
expect(className).toContain(c);
});
expect(className).toContain(baseStencil.modifiers.size.large);

// calling the stencil
type Args = Exclude<Parameters<typeof extendedStencil>[0], undefined>;
Expand Down Expand Up @@ -1115,6 +1145,9 @@ describe('cs', () => {
large: {},
small: {},
},
foo: {
bar: {},
},
},
compound: [
{
Expand Down Expand Up @@ -1142,7 +1175,7 @@ describe('cs', () => {
expect(className).toContain(baseStencil.modifiers.position.only);

// baseStencil.base, baseStencil large, baseStencil only position, baseStencil compound, extendedStencil.base, extendedStencil large, extendedStencil compound
expect(className.split(' ')).toHaveLength(7);
expect(className.split(' ')).toHaveLength(10); // 7 + 3 modifier hashes
});
});
});
Expand Down

0 comments on commit 78514b9

Please sign in to comment.