Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix the cx() function's class collection at runtime as it generated class instances rather than strings. #1697

Merged
merged 2 commits into from
Jul 21, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/friendly-eels-cover.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@compiled/react': patch
---

Fix the `cx()` function's class collection at runtime as it generated class instances rather than strings
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,8 @@ interface CSSPropertiesSchema {
color: 'var(--ds-text-hover)';
background: 'var(--ds-surface-hover)' | 'var(--ds-surface-sunken-hover)';
};
color: 'var(--ds-text)' | 'var(--ds-text-bold)';
background: 'var(--ds-surface)' | 'var(--ds-surface-sunken)';
color: 'var(--ds-text)' | 'var(--ds-text-bold)' | 'var(--ds-text-error)';
background: 'var(--ds-surface)' | 'var(--ds-surface-sunken)' | 'var(--ds-surface-overlay)';
bkgrnd: 'red' | 'green';
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,95 @@
/** @jsxImportSource @compiled/react */
import { render } from '@testing-library/react';

import { cssMap, type XCSSProp, cx } from './__fixtures__/strict-api';

const styles = cssMap({
rootNative: {
color: 'var(--ds-text)',
background: 'var(--ds-surface)',
},
rootComponent: {
color: 'var(--ds-text-error)',
background: 'var(--ds-surface-overlay)',
},
bold: {
color: 'var(--ds-text-bold)',
},
sunken: {
background: 'var(--ds-surface-sunken)',
},
});

function ComponentPassThrough({
xcss,
}: {
xcss?: ReturnType<typeof XCSSProp<'background' | 'color', '&:hover'>>;
}) {
return <NativePassThrough xcss={cx(styles.rootComponent, xcss)} />;
}

function NativePassThrough({
xcss,
}: {
xcss?: ReturnType<typeof XCSSProp<'background' | 'color', '&:hover'>>;
}) {
return <button data-testid="button" className={xcss} css={styles.rootNative} />;
}

describe('pass-through props.xcss directly to DOM', () => {
it('works with no props.xcss', () => {
const { getByTestId } = render(<NativePassThrough />);

expect(getByTestId('button')).toHaveCompiledCss({
color: 'var(--ds-text)',
background: 'var(--ds-surface)',
});
});

it('works with pass-through props.xcss', () => {
const { getByTestId } = render(<NativePassThrough xcss={styles.bold} />);

expect(getByTestId('button')).toHaveCompiledCss({
color: 'var(--ds-text-bold)',
background: 'var(--ds-surface)', // rootNative styles
});
});

it('works with pass-through multiple props.xcss via cx', () => {
const { getByTestId } = render(<NativePassThrough xcss={cx(styles.bold, styles.sunken)} />);

expect(getByTestId('button')).toHaveCompiledCss({
color: 'var(--ds-text-bold)',
background: 'var(--ds-surface-sunken)',
});
});
});

describe('pass-through props.xcss via another component', () => {
it('works with no props.xcss', () => {
const { getByTestId } = render(<ComponentPassThrough />);

expect(getByTestId('button')).toHaveCompiledCss({
color: 'var(--ds-text-error)',
background: 'var(--ds-surface-overlay)',
});
});

it('works with pass-through props.xcss', () => {
const { getByTestId } = render(<ComponentPassThrough xcss={styles.bold} />);

expect(getByTestId('button')).toHaveCompiledCss({
color: 'var(--ds-text-bold)',
background: 'var(--ds-surface-overlay)', // rootComponent styles
});
});

it('works with pass-through multiple props.xcss via cx', () => {
const { getByTestId } = render(<ComponentPassThrough xcss={cx(styles.bold, styles.sunken)} />);

expect(getByTestId('button')).toHaveCompiledCss({
color: 'var(--ds-text-bold)',
background: 'var(--ds-surface-sunken)',
});
});
});
4 changes: 2 additions & 2 deletions packages/react/src/xcss-prop/index.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import type * as CSS from 'csstype';

import type { ApplySchemaValue } from '../create-strict-api/types';
import { ac } from '../runtime';
import { ax } from '../runtime';
import type { CSSPseudos, CSSPseudoClasses, CSSProperties, StrictCSSProperties } from '../types';

type MarkAsRequired<T, K extends keyof T> = T & { [P in K]-?: T[P] };
Expand Down Expand Up @@ -208,5 +208,5 @@ export const cx = <TStyles extends [...XCSSProp<any, any>[]]>(

// The output should be a union type of passed in styles. This ensures the call
// site of xcss prop can raise violations when disallowed styles have been passed.
return ac(actualStyles) as TStyles[number];
return ax(actualStyles) as TStyles[number];
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm worried this doesn't fail tests, not sure how to test this to be honest. I don't think anything is testing this properly (else it would've been failing before).

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

a setup like that in packages/react/src/create-strict-api/__tests__/css-func.test.tsx could work for testing it right?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought this would've failed in this test before to be frank:

const { getByTestId } = render(<Button xcss={cx(stylesOne.primary, stylesTwo.hover)} />);
, but I think because it was passing to the root <div className={xcss}> it wasn't a problem, it's when it's wrapped in <Component xcss={xcss}> it becomes a problem…?

Added tests in f0f1899 and can confirm, without this ax diff it breaks:
Screenshot 2024-07-19 at 1 18 44 PM

};
Loading