diff --git a/.changeset/dirty-carrots-sit.md b/.changeset/dirty-carrots-sit.md new file mode 100644 index 000000000..336a8a595 --- /dev/null +++ b/.changeset/dirty-carrots-sit.md @@ -0,0 +1,13 @@ +--- +'@compiled/babel-plugin': minor +--- + +Fix `@compiled/babel-plugin` to not require `cssMap()` to be called prior to use. + +Example, this failed before for no reason other than the fact that our `state.cssMap` was generated _after_ `JSXElement` and `JSXOpeningElement` were ran. + +```tsx +import { cssMap } from '@compiled/react'; +export default () =>
; +const styles = cssMap({ root: { padding: 8 } }); +``` diff --git a/.changeset/silent-geese-wonder.md b/.changeset/silent-geese-wonder.md new file mode 100644 index 000000000..ffeef80ee --- /dev/null +++ b/.changeset/silent-geese-wonder.md @@ -0,0 +1,21 @@ +--- +'@compiled/babel-plugin': minor +--- + +Throw an error when compiling a `cssMap` object where we expect a `css` or nested `cssMap` object. + +Example of code that silently fails today, using `styles` directly: + +```tsx +import { cssMap } from '@compiled/react'; +const styles = cssMap({ root: { padding: 8 } }); +export default () =>
; +``` + +What we expect to see instead, using `styles.root` instead: + +```tsx +import { cssMap } from '@compiled/react'; +const styles = cssMap({ root: { padding: 8 } }); +export default () =>
; +``` diff --git a/packages/babel-plugin/src/babel-plugin.ts b/packages/babel-plugin/src/babel-plugin.ts index 380101199..89d03245b 100644 --- a/packages/babel-plugin/src/babel-plugin.ts +++ b/packages/babel-plugin/src/babel-plugin.ts @@ -76,6 +76,7 @@ export default declare((api) => { this.sheets = {}; this.cssMap = {}; + this.ignoreMemberExpressions = {}; let cache: Cache; if (this.opts.cache === true) { @@ -239,7 +240,6 @@ export default declare((api) => { }, ImportDeclaration(path, state) { const userLandModule = path.node.source.value; - const isCompiledModule = this.importSources.some((compiledModuleOrigin) => { if (compiledModuleOrigin === userLandModule) { return true; @@ -282,6 +282,7 @@ export default declare((api) => { const apiArray = state.compiledImports[apiName] || []; apiArray.push(specifier.node.local.name); state.compiledImports[apiName] = apiArray; + specifier.remove(); } }); @@ -311,6 +312,7 @@ Reasons this might happen: path.parentPath ); } + if (isCompiledCSSMapCallExpression(path.node, state)) { visitCssMapPath(path, { context: 'root', state, parentPath: path }); return; diff --git a/packages/babel-plugin/src/css-map/__tests__/index.test.ts b/packages/babel-plugin/src/css-map/__tests__/index.test.ts index 7036eecc4..685d60bf5 100644 --- a/packages/babel-plugin/src/css-map/__tests__/index.test.ts +++ b/packages/babel-plugin/src/css-map/__tests__/index.test.ts @@ -36,6 +36,25 @@ describe('css map basic functionality', () => { ]); }); + it('should transform css map even when the styles are defined below the component', () => { + const actual = transform(` + import { cssMap } from '@compiled/react'; + + const Component = () =>
+ + +
+ + const styles = cssMap(${styles}); + `); + + expect(actual).toIncludeMultiple([ + 'const styles={danger:"_syaz5scu _bfhk5scu",success:"_syazbf54 _bfhkbf54"};', + '', + '', + ]); + }); + it('should transform css map even with an empty object', () => { const actual = transform(` import { css, cssMap } from '@compiled/react'; @@ -88,6 +107,30 @@ describe('css map basic functionality', () => { ]); }); + it('should error out if the root cssMap object is being directly called', () => { + expect(() => { + transform(` + import { cssMap } from '@compiled/react'; + + const styles = cssMap({ root: { color: 'red' } }); + + // Eg. we expect 'styles.root' here instead of 'styles' +
+ `); + }).toThrow(ErrorMessages.USE_VARIANT_OF_CSS_MAP); + + expect(() => { + transform(` + import { cssMap } from '@compiled/react'; + + // Eg. we expect 'styles.root' here instead of 'styles' +
+ + const styles = cssMap({ root: { color: 'red' } }); + `); + }).toThrow(ErrorMessages.USE_VARIANT_OF_CSS_MAP); + }); + it('should error out if variants are not defined at the top-most scope of the module.', () => { expect(() => { transform(` diff --git a/packages/babel-plugin/src/types.ts b/packages/babel-plugin/src/types.ts index f697dda57..55c4726e2 100644 --- a/packages/babel-plugin/src/types.ts +++ b/packages/babel-plugin/src/types.ts @@ -194,6 +194,11 @@ export interface State extends PluginPass { */ cssMap: Record; + /** + * Holdings a record of member expression names to ignore + */ + ignoreMemberExpressions: Record; + /** * A custom resolver used to statically evaluate import declarations */ diff --git a/packages/babel-plugin/src/utils/css-builders.ts b/packages/babel-plugin/src/utils/css-builders.ts index f0d6d6f0b..a82253d21 100644 --- a/packages/babel-plugin/src/utils/css-builders.ts +++ b/packages/babel-plugin/src/utils/css-builders.ts @@ -1,15 +1,19 @@ import generate from '@babel/generator'; +import type { NodePath } from '@babel/traverse'; import * as t from '@babel/types'; import { addUnitIfNeeded, cssAffixInterpolation } from '@compiled/css'; import { hash, kebabCase } from '@compiled/utils'; +import { visitCssMapPath } from '../css-map'; import type { Metadata } from '../types'; import { buildCodeFrameError } from './ast'; import { CONDITIONAL_PATHS } from './constants'; +import { createErrorMessage, ErrorMessages } from './css-map'; import { evaluateExpression } from './evaluate-expression'; import { isCompiledCSSCallExpression, + isCompiledCSSMapCallExpression, isCompiledCSSTaggedTemplateExpression, isCompiledKeyframesCallExpression, isCompiledKeyframesTaggedTemplateExpression, @@ -665,6 +669,45 @@ const extractObjectExpression = (node: t.ObjectExpression, meta: Metadata): CSSO return { css: mergeSubsequentUnconditionalCssItems(css), variables }; }; +/** + * If we don't yet have a `meta.state.cssMap[node.name]` built yet, try to build and cache it, eg. in this scenario: + * ```tsx + * const Component = () =>
+ * const styles = cssMap({ root: { padding: 0 } }); + * ``` + * + * If we don't find this is a `cssMap()` call, we put it into `ignoreMemberExpressions` to ignore on future runs. + * + * @returns {Boolean} Whether the cache was generated + */ +const generateCacheForCSSMap = (node: t.Identifier, meta: Metadata): void => { + if (meta.state.cssMap[node.name] || meta.state.ignoreMemberExpressions[node.name]) { + return; + } + + const resolved = resolveBinding(node.name, meta, evaluateExpression); + if (resolved && isCompiledCSSMapCallExpression(resolved.node, meta.state)) { + let resolvedCallPath = resolved.path.get('init'); + if (Array.isArray(resolvedCallPath)) { + resolvedCallPath = resolvedCallPath[0]; + } + + if (t.isCallExpression(resolvedCallPath.node)) { + // This visits the cssMap path and caches the styles + visitCssMapPath(resolvedCallPath as NodePath, { + context: 'root', + state: meta.state, + parentPath: resolved.path, + }); + } + } + + if (!meta.state.cssMap[node.name]) { + // If this cannot be found, it's likely not a `cssMap` identifier and we shouldn't parse it again on future runs… + meta.state.ignoreMemberExpressions[node.name] = true; + } +}; + /** * Extracts CSS data from a member expression node (eg. `styles.primary`) * @@ -688,11 +731,16 @@ function extractMemberExpression( fallbackToEvaluate = true ): CSSOutput | undefined { const bindingIdentifier = findBindingIdentifier(node); - if (bindingIdentifier && meta.state.cssMap[bindingIdentifier.name]) { - return { - css: [{ type: 'map', expression: node, name: bindingIdentifier.name, css: '' }], - variables: [], - }; + + if (bindingIdentifier) { + // In some cases, the `state.cssMap` is not warmed yet, so run it: + generateCacheForCSSMap(bindingIdentifier, meta); + if (meta.state.cssMap[bindingIdentifier.name]) { + return { + css: [{ type: 'map', expression: node, name: bindingIdentifier.name, css: '' }], + variables: [], + }; + } } if (fallbackToEvaluate) { @@ -956,6 +1004,16 @@ export const buildCss = (node: t.Expression | t.Expression[], meta: Metadata): C ); } + // In some cases, the `state.cssMap` is not warmed yet, so run it: + generateCacheForCSSMap(node, meta); + if (meta.state.cssMap[node.name]) { + throw buildCodeFrameError( + createErrorMessage(ErrorMessages.USE_VARIANT_OF_CSS_MAP), + node, + meta.parentPath + ); + } + const result = buildCss(resolvedBinding.node, resolvedBinding.meta); assertNoImportedCssVariables(node, meta, resolvedBinding, result); diff --git a/packages/babel-plugin/src/utils/css-map.ts b/packages/babel-plugin/src/utils/css-map.ts index 2a9786d85..984bcbb0a 100644 --- a/packages/babel-plugin/src/utils/css-map.ts +++ b/packages/babel-plugin/src/utils/css-map.ts @@ -86,6 +86,7 @@ export enum ErrorMessages { STATIC_PROPERTY_KEY = 'Property key may only be a static string.', SELECTOR_BLOCK_WRONG_PLACE = '`selector` key was defined in the wrong place.', USE_SELECTORS_WITH_AMPERSAND = 'This selector is applied to the parent element, and so you need to specify the ampersand symbol (&) directly before it. For example, `:hover` should be written as `&:hover`.', + USE_VARIANT_OF_CSS_MAP = 'You must use the variant of a CSS Map object (eg. `styles.root`), not the root object itself, eg. `styles`.', } export const createErrorMessage = (message: string): string => { diff --git a/packages/react/src/create-strict-api/__tests__/generics.test.tsx b/packages/react/src/create-strict-api/__tests__/generics.test.tsx index 268f8d263..dde15c333 100644 --- a/packages/react/src/create-strict-api/__tests__/generics.test.tsx +++ b/packages/react/src/create-strict-api/__tests__/generics.test.tsx @@ -19,7 +19,7 @@ describe('createStrictAPI()', () => { }); it('should mark all styles as optional in cssMap()', () => { - const styles = cssMap({ + const stylesMap = cssMap({ nested: { '&:hover': {}, '&:active': {}, @@ -28,7 +28,7 @@ describe('createStrictAPI()', () => { }, }); - const { getByTestId } = render(
); + const { getByTestId } = render(
); expect(getByTestId('div')).toBeDefined(); }); @@ -96,7 +96,7 @@ describe('createStrictAPI()', () => { }); it('should violate types for cssMap()', () => { - const styles = cssMap({ + const stylesMap = cssMap({ primary: { // @ts-expect-error — Type '""' is not assignable to type ... color: '', @@ -129,7 +129,7 @@ describe('createStrictAPI()', () => { }, }); - const { getByTestId } = render(
); + const { getByTestId } = render(
); expect(getByTestId('div')).toBeDefined(); }); @@ -224,7 +224,7 @@ describe('createStrictAPI()', () => { }); it('should pass type check for cssMap()', () => { - const styles = cssMap({ + const stylesMap = cssMap({ primary: { color: 'var(--ds-text)', backgroundColor: 'var(--ds-bold)', @@ -258,7 +258,7 @@ describe('createStrictAPI()', () => { }, }); - const { getByTestId } = render(
); + const { getByTestId } = render(
); expect(getByTestId('div')).toHaveCompiledCss('color', 'var(--ds-text)'); });