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

cloneElement support #1729

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from
Draft
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
89 changes: 89 additions & 0 deletions packages/babel-plugin/src/__tests__/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,95 @@ describe('babel plugin', () => {
`);
});

it('should transform cloneElement css prop', () => {
const actual = transform(`
import { cloneElement } from 'react';
import { css } from '@compiled/react';

const MyDiv = ({ children }) => {
return cloneElement(children, { css: css({ fontSize: 12 }) });
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is would be the full example of what we need this for—do you think it would work?

const styles = css({ fontSize: 12 });
const MyDiv = ({ children, className }) => {
  return cloneElement(children, { css: cx(styles, props.className) });
}

Copy link
Collaborator

Choose a reason for hiding this comment

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

The other question I've got, probably for Compiled, is should this be props.css or props.className exclusively?

Copy link
Collaborator Author

@danieldelcore danieldelcore Oct 23, 2024

Choose a reason for hiding this comment

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

I had the same question.

I'll add a test case for the example you provided! It should just work because that part of the algorithm gets handed back to compiled

};
`);

expect(actual).toMatchInlineSnapshot(`
"/* File generated by @compiled/babel-plugin v0.0.0 */
import * as React from "react";
import { ax, ix, CC, CS } from "@compiled/react/runtime";
import { cloneElement } from "react";
const _ = "._1wyb1fwx{font-size:12px}";
const MyDiv = ({ children }) => {
return (
<CC>
<CS>{[_]}</CS>
{cloneElement(children, {
className: ax(["_1wyb1fwx"]),
})}
</CC>
);
};
"
`);
});

it('should transform cloneElement css prop with aliased import', () => {
const actual = transform(`
import { cloneElement as CE } from 'react';
import { css } from '@compiled/react';

const MyDiv = ({ children }) => {
return CE(children, { css: css({ fontSize: 12 }) });
};
`);

expect(actual).toMatchInlineSnapshot(`
"/* File generated by @compiled/babel-plugin v0.0.0 */
import * as React from "react";
import { ax, ix, CC, CS } from "@compiled/react/runtime";
import { cloneElement as CE } from "react";
const _ = "._1wyb1fwx{font-size:12px}";
const MyDiv = ({ children }) => {
return (
<CC>
<CS>{[_]}</CS>
{CE(children, {
className: ax(["_1wyb1fwx"]),
})}
</CC>
);
};
"
`);
});

it('should transform React.cloneElement css prop', () => {
const actual = transform(`
import React from 'react';
import { css } from '@compiled/react';

const MyDiv = ({ children }) => {
return React.cloneElement(children, { css: css({ fontSize: 12 }) });
};
`);

expect(actual).toMatchInlineSnapshot(`
"/* File generated by @compiled/babel-plugin v0.0.0 */
import { ax, ix, CC, CS } from "@compiled/react/runtime";
import React from "react";
const _ = "._1wyb1fwx{font-size:12px}";
const MyDiv = ({ children }) => {
return (
<CC>
<CS>{[_]}</CS>
{React.cloneElement(children, {
className: ax(["_1wyb1fwx"]),
})}
</CC>
);
};
"
`);
});

// TODO Removing import React from 'react' breaks this test
it('should preserve comments at the top of the processed file before inserting runtime imports', () => {
const actual = transform(`
Expand Down
42 changes: 42 additions & 0 deletions packages/babel-plugin/src/babel-plugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import {
} from '@compiled/utils';

import { visitClassNamesPath } from './class-names';
import { visitCloneElementPath } from './clone-element';
import { visitCssMapPath } from './css-map';
import { visitCssPropPath } from './css-prop';
import { visitStyledPath } from './styled';
Expand Down Expand Up @@ -65,6 +66,26 @@ const findClassicJsxPragmaImport: Visitor<State> = {
},
};

const findReactImportSpecifier: Visitor<State> = {
ImportSpecifier(path, state) {
const specifier = path.node;

t.assertImportDeclaration(path.parent);
if (path.parent.source.value !== 'react') {
return;
}

if (
(specifier.imported.type === 'StringLiteral' &&
specifier.imported.value === 'cloneElement') ||
(specifier.imported.type === 'Identifier' && specifier.imported.name === 'cloneElement')
) {
state.reactImports = state.reactImports || {};
state.reactImports.cloneElement = specifier.local.name;
}
},
};

export default declare<State>((api) => {
api.assertVersion(7);

Expand Down Expand Up @@ -124,6 +145,7 @@ export default declare<State>((api) => {

// Handle classic JSX pragma, if it exists
path.traverse<State>(findClassicJsxPragmaImport, this);
path.traverse<State>(findReactImportSpecifier, this);

if (!file.ast.comments) {
return;
Expand Down Expand Up @@ -295,6 +317,26 @@ export default declare<State>((api) => {
path: NodePath<t.TaggedTemplateExpression> | NodePath<t.CallExpression>,
state: State
) {
if (
Copy link
Collaborator

Choose a reason for hiding this comment

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

should this check that cloneElement is imported from react too?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes definitely and also should check for aliased imports

import { cloneElement as foo } from 'react';

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Edit: I added support for alias and member expressions in the latest commit

(t.isCallExpression(path.node) &&
t.isIdentifier(path.node.callee) &&
path.node.callee.name === state.reactImports?.cloneElement) ||
// handle member expression React.cloneElement
(t.isCallExpression(path.node) &&
t.isMemberExpression(path.node.callee) &&
t.isIdentifier(path.node.callee.object) &&
path.node.callee.object.name === 'React' &&
t.isIdentifier(path.node.callee.property) &&
path.node.callee.property.name === 'cloneElement')
) {
visitCloneElementPath(path as NodePath<t.CallExpression>, {
context: 'root',
state,
parentPath: path,
});
return;
}

if (isTransformedJsxFunction(path, state)) {
throw buildCodeFrameError(
`Found a \`jsx\` function call in the Babel output where one should not have been generated. Was Compiled not set up correctly?
Expand Down
131 changes: 131 additions & 0 deletions packages/babel-plugin/src/clone-element/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,131 @@
import type { NodePath } from '@babel/core';
import * as t from '@babel/types';

import type { Metadata } from '../types';
import { buildCompiledCloneElement } from '../utils/build-compiled-component';
import { buildCss } from '../utils/css-builders';
import { getRuntimeClassNameLibrary } from '../utils/get-runtime-class-name-library';
import { resolveIdentifierComingFromDestructuring } from '../utils/resolve-binding';
import { transformCssItems } from '../utils/transform-css-items';
import type { CSSOutput } from '../utils/types';

/**
* Extracts styles from an expression.
*
* @param path Expression node
*/
const extractStyles = (path: NodePath<t.Expression>): t.Expression[] | t.Expression | undefined => {
if (
t.isCallExpression(path.node) &&
t.isIdentifier(path.node.callee) &&
path.node.callee.name === 'css' &&
t.isExpression(path.node.arguments[0])
) {
// css({}) call
const styles = path.node.arguments as t.Expression[];
return styles;
}

if (
t.isCallExpression(path.node) &&
t.isIdentifier(path.node.callee) &&
t.isExpression(path.node.arguments[0]) &&
path.scope.hasOwnBinding(path.node.callee.name)
) {
const binding = path.scope.getBinding(path.node.callee.name)?.path.node;

if (
!!resolveIdentifierComingFromDestructuring({ name: 'css', node: binding as t.Expression })
) {
// c({}) rename call
const styles = path.node.arguments as t.Expression[];
return styles;
}
}

if (t.isCallExpression(path.node) && t.isMemberExpression(path.node.callee)) {
if (
t.isIdentifier(path.node.callee.property) &&
path.node.callee.property.name === 'css' &&
t.isExpression(path.node.arguments[0])
) {
// props.css({}) call
const styles = path.node.arguments as t.Expression[];
return styles;
}
}

if (t.isTaggedTemplateExpression(path.node)) {
const styles = path.node.quasi;
return styles;
}

return undefined;
};

/**
* Takes a React.cloneElement invocation and transforms it into a compiled component.
* This method will traverse the AST twice,
* once to replace all calls to `css`,
* and another to replace `style` usage.
*
* `React.cloneElement(<Component />, { css: {} })`
*
* @param path {NodePath} The opening JSX element
* @param meta {Metadata} Useful metadata that can be used during the transformation
*/
export const visitCloneElementPath = (path: NodePath<t.CallExpression>, meta: Metadata): void => {
// if props contains a `css` prop, we need to transform it.
const props = path.node.arguments[1];

if (props.type !== 'ObjectExpression') {
// TODO: handle this case properly
console.error('cloneElement props are not an ObjectExpression');
return;
}

const collectedVariables: CSSOutput['variables'] = [];
const collectedSheets: string[] = [];

// First pass to replace all usages of `css({})`
path.traverse({
CallExpression(path) {
const styles = extractStyles(path);

if (!styles) {
// Nothing to do - skip.
return;
}

const builtCss = buildCss(styles, meta);
const { sheets, classNames } = transformCssItems(builtCss.css, meta);

collectedVariables.push(...builtCss.variables);
collectedSheets.push(...sheets);

path.replaceWith(
t.callExpression(t.identifier(getRuntimeClassNameLibrary(meta)), [
t.arrayExpression(classNames),
])
);

// find ancestor cloneElement callExpression
const ancestorPath = path.findParent(
(p) =>
(p.isCallExpression() &&
t.isIdentifier(p.node.callee) &&
p.node.callee.name === meta.state.reactImports?.cloneElement) ||
(p.isCallExpression() &&
t.isMemberExpression(p.node.callee) &&
t.isIdentifier(p.node.callee.property) &&
p.node.callee.property.name === 'cloneElement')
) as NodePath<t.CallExpression>;

if (!ancestorPath) {
return;
}

ancestorPath.replaceWith(buildCompiledCloneElement(ancestorPath.node, builtCss, meta));
},
});
};
16 changes: 16 additions & 0 deletions packages/babel-plugin/src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,22 @@ export interface State extends PluginPass {
cssMap?: string[];
};

/**
* Returns the name of the cloneElement import specifier if it is imported.
* If an alias is used, the alias will be returned.
*
* E.g:
*
* ```
* import { cloneElement as myCloneElement } from 'react';
* ```
*
* Returns `myCloneElement`.
*/
reactImports?: {
cloneElement?: string;
};

usesXcss?: boolean;

importedCompiledImports?: {
Expand Down
58 changes: 58 additions & 0 deletions packages/babel-plugin/src/utils/build-compiled-component.ts
Original file line number Diff line number Diff line change
Expand Up @@ -142,3 +142,61 @@ export const buildCompiledComponent = (

return compiledTemplate(node, sheets, meta);
};

/**
* Accepts a cloneElement node and returns a Compiled Component AST.
*
* @param node Originating cloneElement node
* @param cssOutput CSS and variables to place onto the component
* @param meta {Metadata} Useful metadata that can be used during the transformation
*/
export const buildCompiledCloneElement = (
node: t.CallExpression,
cssOutput: CSSOutput,
meta: Metadata
): t.Node => {
const { sheets, classNames } = transformCssItems(cssOutput.css, meta);

const props = node.arguments[1];

// TODO: This is a temporary fix to prevent the plugin from crashing when the second argument of cloneElement is not an object expression.
if (!t.isObjectExpression(props)) {
throw new Error('Second argument of cloneElement must be an object expression.');
}
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

If someone is using a variable/spread/etc it'll probably be impossible to reliably fish out and modify the CSS prop.
Are there any standard approaches to this problem in use in the repo already? Do we throw/Log an err/try really hard to locate the variable?


const [classNameProperty] = props.properties.filter(
(prop) => t.isObjectProperty(prop) && t.isIdentifier(prop.key) && prop.key.name === 'className'
);

if (
classNameProperty &&
t.isObjectProperty(classNameProperty) &&
t.isIdentifier(classNameProperty.value)
) {
const values: t.Expression[] = classNames.concat(classNameProperty.value);

classNameProperty.value = t.callExpression(t.identifier(getRuntimeClassNameLibrary(meta)), [
t.arrayExpression(values),
]);
} else {
props.properties.push(
t.objectProperty(
t.identifier('className'),
t.callExpression(t.identifier(getRuntimeClassNameLibrary(meta)), [
t.arrayExpression(classNames),
])
)
);
}

// remove css prop from props object
const cssPropIndex = props.properties.findIndex(
(prop) => t.isObjectProperty(prop) && t.isIdentifier(prop.key) && prop.key.name === 'css'
);

if (cssPropIndex !== -1) {
props.properties.splice(cssPropIndex, 1);
}

return compiledTemplate(node, sheets, meta);
};
Loading