From dccb71e0e825db8aae4df6b41a7cb6094642e53d Mon Sep 17 00:00:00 2001 From: Michael Dougall <6801309+itsdouges@users.noreply.github.com> Date: Thu, 26 Oct 2023 16:20:43 +1100 Subject: [PATCH] XCSS prop options arg (#1541) * fix: Fixes empty inline object throwing * feat: add type helpers to flag properties and pseudos as required * chore: update * chore: update jsdoc --- .changeset/purple-flowers-draw.md | 6 ++ .../__tests__/transformation.test.ts | 18 +++++ packages/babel-plugin/src/xcss-prop/index.ts | 23 +++++- .../xcss-prop/__tests__/xcss-prop.test.tsx | 76 ++++++++++++++++++- packages/react/src/xcss-prop/index.ts | 47 +++++++++--- 5 files changed, 156 insertions(+), 14 deletions(-) create mode 100644 .changeset/purple-flowers-draw.md diff --git a/.changeset/purple-flowers-draw.md b/.changeset/purple-flowers-draw.md new file mode 100644 index 000000000..21d982188 --- /dev/null +++ b/.changeset/purple-flowers-draw.md @@ -0,0 +1,6 @@ +--- +'@compiled/babel-plugin': patch +'@compiled/react': patch +--- + +Adds third generic for XCSSProp type for declaring what properties and pseudos should be required. diff --git a/packages/babel-plugin/src/xcss-prop/__tests__/transformation.test.ts b/packages/babel-plugin/src/xcss-prop/__tests__/transformation.test.ts index 0554c7d56..2b59cfdaa 100644 --- a/packages/babel-plugin/src/xcss-prop/__tests__/transformation.test.ts +++ b/packages/babel-plugin/src/xcss-prop/__tests__/transformation.test.ts @@ -190,4 +190,22 @@ describe('xcss prop transformation', () => { " `); }); + + it('should not blow up transforming an empty xcss object', () => { + const result = transform( + ` + + ` + ); + + expect(result).toMatchInlineSnapshot(` + "import * as React from "react"; + import { ax, ix, CC, CS } from "@compiled/react/runtime"; + + {[]} + {} + ; + " + `); + }); }); diff --git a/packages/babel-plugin/src/xcss-prop/index.ts b/packages/babel-plugin/src/xcss-prop/index.ts index f186348f9..e9708d55e 100644 --- a/packages/babel-plugin/src/xcss-prop/index.ts +++ b/packages/babel-plugin/src/xcss-prop/index.ts @@ -69,9 +69,26 @@ export const visitXcssPropPath = (path: NodePath, meta: Met const cssOutput = buildCss(container.expression, meta); const { sheets, classNames } = transformCssItems(cssOutput.css, meta); - // Replace xcss prop with class names - // The object has a type constraint to always be a basic object with no values. - container.expression = classNames[0]; + switch (classNames.length) { + case 1: + // Replace xcss prop with class names + // Remeber: The object has a type constraint to always be a basic object with no values. + container.expression = classNames[0]; + break; + + case 0: + // No styles were merged so we replace with an undefined identifier. + container.expression = t.identifier('undefined'); + break; + + default: + throw buildCodeFrameError( + 'Unexpected count of class names please raise an issue on Github', + prop.node, + meta.parentPath + ); + } + path.parentPath.replaceWith(compiledTemplate(jsxElementNode, sheets, meta)); } else { const sheets = collectPassStyles(meta); diff --git a/packages/react/src/xcss-prop/__tests__/xcss-prop.test.tsx b/packages/react/src/xcss-prop/__tests__/xcss-prop.test.tsx index a8b302840..f7bdff96d 100644 --- a/packages/react/src/xcss-prop/__tests__/xcss-prop.test.tsx +++ b/packages/react/src/xcss-prop/__tests__/xcss-prop.test.tsx @@ -205,7 +205,79 @@ describe('xcss prop', () => { }, }); - // @ts-expect-error — Type 'CompiledStyles<{ selectors: { '&:not(:first-child):last-child': { color: "red"; }; }; }>' is not assignable to type 'XCSSProp<"color", "&:hover">'. - expectTypeOf().toBeObject(); + expectTypeOf( + ' is not assignable to type 'undefined'. + xcss={styles.primary} + /> + ).toBeObject(); + }); + + it('should mark a xcss prop as required', () => { + function CSSPropComponent({ + xcss, + }: { + xcss: XCSSProp< + 'color' | 'backgroundColor', + '&:hover', + { requiredProperties: 'color'; requiredPseudos: never } + >; + }) { + return
foo
; + } + + expectTypeOf( + '. + xcss={{}} + /> + ).toBeObject(); + }); + + it('should mark a xcss prop inside a pseudo as required', () => { + function CSSPropComponent({ + xcss, + }: { + xcss: XCSSProp< + 'color' | 'backgroundColor', + '&:hover', + { requiredProperties: 'color'; requiredPseudos: never } + >; + }) { + return
foo
; + } + + expectTypeOf( + + ).toBeObject(); + }); + + it('should mark a xcss prop pseudo as required', () => { + function CSSPropComponent({ + xcss, + }: { + xcss: XCSSProp< + 'color' | 'backgroundColor', + '&:hover', + { requiredProperties: never; requiredPseudos: '&:hover' } + >; + }) { + return
foo
; + } + + expectTypeOf( + , never>; }'. + xcss={{ + color: 'red', + }} + /> + ).toBeObject(); }); }); diff --git a/packages/react/src/xcss-prop/index.ts b/packages/react/src/xcss-prop/index.ts index 282ff6960..6fa27a52e 100644 --- a/packages/react/src/xcss-prop/index.ts +++ b/packages/react/src/xcss-prop/index.ts @@ -9,15 +9,28 @@ type XCSSItem = { : never; }; -type XCSSPseudos = { - [Q in CSSPseudos]?: Q extends TPseudos ? XCSSItem : never; +type XCSSPseudos< + TAllowedProperties extends keyof CSSProperties, + TAllowedPseudos extends CSSPseudos, + TRequiredProperties extends { requiredProperties: TAllowedProperties } +> = { + [Q in CSSPseudos]?: Q extends TAllowedPseudos + ? MarkAsRequired, TRequiredProperties['requiredProperties']> + : never; }; /** - * We currently block all at rules from xcss prop. - * This needs us to decide on what the final API is across Compiled to be able to set. + * These APIs we don't want to allow to be passed through the `xcss` prop but we also + * must declare them so the (lack-of a) excess property check doesn't bite us and allow + * unexpected values through. */ -type XCSSAtRules = { +type BlockedRules = { + selectors?: never; +} & { + /** + * We currently block all at rules from xcss prop. + * This needs us to decide on what the final API is across Compiled to be able to set. + */ [Q in CSS.AtRules]?: never; }; @@ -68,10 +81,12 @@ export type XCSSAllPseudos = CSSPseudos; * it means products only pay for styles they use as they're now the ones who declare * the styles! * - * The {@link XCSSProp} type has generics which must be defined — of which should be what you - * explicitly want to maintain as API. Use {@link XCSSAllProperties} and {@link XCSSAllPseudos} + * The {@link XCSSProp} type has generics two of which must be defined — use to explicitly + * set want you to maintain as API. Use {@link XCSSAllProperties} and {@link XCSSAllPseudos} * to enable all properties and pseudos. * + * The third generic is used to declare what properties and pseudos should be required. + * * @example * ``` * interface MyComponentProps { @@ -86,6 +101,9 @@ export type XCSSAllPseudos = CSSPseudos; * * // All properties are accepted, only the hover pseudo is accepted. * xcss?: XCSSProp; + * + * // The xcss prop is required as well as the color property. No pseudos are required. + * xcss: XCSSProp; * } * * function MyComponent({ xcss }: MyComponentProps) { @@ -109,13 +127,24 @@ export type XCSSAllPseudos = CSSPseudos; */ export type XCSSProp< TAllowedProperties extends keyof CSSProperties, - TAllowedPseudos extends CSSPseudos + TAllowedPseudos extends CSSPseudos, + TRequiredProperties extends { + requiredProperties: TAllowedProperties; + requiredPseudos: TAllowedPseudos; + } = never > = - | (XCSSItem & XCSSPseudos & XCSSAtRules) + | (MarkAsRequired, TRequiredProperties['requiredProperties']> & + MarkAsRequired< + XCSSPseudos, + TRequiredProperties['requiredPseudos'] + > & + BlockedRules) | false | null | undefined; +type MarkAsRequired = T & { [P in K]-?: T[P] }; + /** * ## cx *