diff --git a/src/sx/src/__tests__/create.test.js b/src/sx/src/__tests__/create.test.js index be4c5af24a..0ad14b06df 100644 --- a/src/sx/src/__tests__/create.test.js +++ b/src/sx/src/__tests__/create.test.js @@ -99,10 +99,9 @@ it('supports conditional calls', () => { expect(styles('button', isEnabled && 'disabled')).toMatchInlineSnapshot(`"_2dHaKY"`); // alternative syntax: + expect(() => styles({ button: false })).not.toThrowError(); + expect(styles({ button: false })).toBeUndefined(); expect(styles({ button: true })).toMatchInlineSnapshot(`"_324Crd"`); - expect(() => styles({ button: false })).toThrowErrorMatchingInlineSnapshot( - `"SX must be called with at least one stylesheet name."`, - ); expect(styles({ button: true, disabled: isDisabled })).toMatchInlineSnapshot(`"_324Crd"`); expect(styles({ button: true, disabled: isEnabled })).toMatchInlineSnapshot(`"_2dHaKY"`); }); @@ -122,14 +121,22 @@ it('validates incorrect usage', () => { aaa: { color: 'red' }, bbb: { color: 'blue' }, }); + + // $FlowExpectedError[incompatible-call] unexpected yadada + expect(() => styles({}, 'yadada')).toThrowErrorMatchingInlineSnapshot( + `"SX accepts only one argument when using conditional objects. Either remove the second argument or switch to traditional syntax without conditional objects."`, + ); + expect(() => styles({})).toThrowErrorMatchingInlineSnapshot( + `"SX must be called with at least one stylesheet selector (empty object given)."`, + ); expect(() => styles()).toThrowErrorMatchingInlineSnapshot( `"SX must be called with at least one stylesheet name."`, ); - // $FlowExpectedError[incompatible-call] ccc + // $FlowExpectedError[incompatible-call] unexpected bbc expect(() => styles('bbc')).toThrowErrorMatchingInlineSnapshot( `"SX was called with 'bbc' stylesheet name but it doesn't exist. Did you mean 'bbb' instead?"`, ); - // $FlowExpectedError[incompatible-call] ccc + // $FlowExpectedError[incompatible-call] unexpected ccc expect(() => styles('ccc')).toThrowErrorMatchingInlineSnapshot( `"SX was called with 'ccc' stylesheet name but it doesn't exist. Did you mean 'aaa' instead?"`, ); diff --git a/src/sx/src/create.js b/src/sx/src/create.js index 0d74c60959..bee807d1bb 100644 --- a/src/sx/src/create.js +++ b/src/sx/src/create.js @@ -38,7 +38,7 @@ function suggest(sheetDefinitionName: string, alternativeNames: Array): type CreateReturnType = { (...$ReadOnlyArray | false>): string, // conditional strings `styles(isRed && 'red')` - ({ +[$Keys]: boolean }): string, // conditional object `styles({ red: isRed })` + ({ +[$Keys]: boolean }): string | void, // conditional object `styles({ red: isRed })` +[$Keys]: AllCSSProperties, // for external styles merging }; @@ -57,15 +57,23 @@ export default function create(sheetDefinitions: T): Create function sxFunction(maybeObject, ...styleSheetsSelectors) { let sheetDefinitionNames; if (isObject(maybeObject)) { + invariant( + styleSheetsSelectors.length === 0, + 'SX accepts only one argument when using conditional objects. Either remove the second argument or switch to traditional syntax without conditional objects.', + ); sheetDefinitionNames = Object.keys(maybeObject).filter((key) => maybeObject[key] === true); + invariant( + isObjectEmpty(maybeObject) === false, + 'SX must be called with at least one stylesheet selector (empty object given).', + ); } else { sheetDefinitionNames = [maybeObject, ...styleSheetsSelectors].filter((el) => el != null); + invariant( + sheetDefinitionNames.length > 0, + 'SX must be called with at least one stylesheet name.', + ); } - invariant( - sheetDefinitionNames.length > 0, - 'SX must be called with at least one stylesheet name.', - ); const selectedStyles = {}; for (const sheetDefinitionName of sheetDefinitionNames) { if (sheetDefinitionName != null && sheetDefinitionName !== false) { @@ -83,6 +91,18 @@ export default function create(sheetDefinitions: T): Create } } const classes = Object.values(selectedStyles); + + if (classes.length === 0 && isObject(maybeObject)) { + // This happens when user is using conditional selectors. It would be incorrect to return an + // empty string because React would render `class=""`. Instead, we want to skip the class + // attribute completely. Example of such situation: + // + // ``` + //
+ // ``` + return undefined; + } + const uniqueClasses = [...new Set(classes)]; return uniqueClasses.join(' '); } @@ -92,5 +112,6 @@ export default function create(sheetDefinitions: T): Create sxFunction[sheetDefinitionKey] = sheetDefinitions[sheetDefinitionKey]; } + // $FlowFixMe[incompatible-return] not sure how to explain that conditional object can return `void` return sxFunction; }