Skip to content

Commit

Permalink
SX: update conditional object invariants
Browse files Browse the repository at this point in the history
Previously, we didn't allow conditional objects resulting in no styles, for example:

```
styles({ button: false })
```

This change modifies the invariant constraints to allow such situations and return `undefined`.
  • Loading branch information
mrtnzlml authored and kodiakhq[bot] committed Jun 8, 2021
1 parent 502d6fa commit 9a58e99
Show file tree
Hide file tree
Showing 2 changed files with 38 additions and 10 deletions.
17 changes: 12 additions & 5 deletions src/sx/src/__tests__/create.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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"`);
});
Expand All @@ -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?"`,
);
Expand Down
31 changes: 26 additions & 5 deletions src/sx/src/create.js
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ function suggest(sheetDefinitionName: string, alternativeNames: Array<string>):

type CreateReturnType<T> = {
(...$ReadOnlyArray<?$Keys<T> | false>): string, // conditional strings `styles(isRed && 'red')`
({ +[$Keys<T>]: boolean }): string, // conditional object `styles({ red: isRed })`
({ +[$Keys<T>]: boolean }): string | void, // conditional object `styles({ red: isRed })`
+[$Keys<T>]: AllCSSProperties, // for external styles merging
};

Expand All @@ -57,15 +57,23 @@ export default function create<T: SheetDefinitions>(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) {
Expand All @@ -83,6 +91,18 @@ export default function create<T: SheetDefinitions>(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:
//
// ```
// <div className={styles({ conditionalStyle: false })} />
// ```
return undefined;
}

const uniqueClasses = [...new Set(classes)];
return uniqueClasses.join(' ');
}
Expand All @@ -92,5 +112,6 @@ export default function create<T: SheetDefinitions>(sheetDefinitions: T): Create
sxFunction[sheetDefinitionKey] = sheetDefinitions[sheetDefinitionKey];
}

// $FlowFixMe[incompatible-return] not sure how to explain that conditional object can return `void`
return sxFunction;
}

0 comments on commit 9a58e99

Please sign in to comment.