Skip to content

Commit

Permalink
Fix identifiers shadowed by a function argument (#810)
Browse files Browse the repository at this point in the history
Use the correct expression in the style prop when an identifier is
shadowed by a function argument, by ensuring values set by a previous
meta object are not re-used
  • Loading branch information
MonicaOlejniczak authored Aug 12, 2021
1 parent 5703722 commit 48805ec
Show file tree
Hide file tree
Showing 5 changed files with 78 additions and 12 deletions.
5 changes: 5 additions & 0 deletions .changeset/tame-frogs-impress.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@compiled/babel-plugin': patch
---

Use the correct expression in the style prop, when an identifier is shadowed by a function argument
3 changes: 2 additions & 1 deletion packages/babel-plugin/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@
"@types/babel__core": "^7.1.15",
"@types/benchmark": "^2.1.1",
"@types/resolve": "^1.20.1",
"benchmark": "^2.1.4"
"benchmark": "^2.1.4",
"prettier": "^2.2.1"
}
}
50 changes: 40 additions & 10 deletions packages/babel-plugin/src/__tests__/css-builder.test.tsx
Original file line number Diff line number Diff line change
@@ -1,13 +1,4 @@
import { transformSync } from '@babel/core';
import babelPlugin from '../index';

const transform = (code: string) => {
return transformSync(code, {
configFile: false,
babelrc: false,
plugins: [babelPlugin],
})?.code;
};
import { transform } from './test-utils';

describe('css builder', () => {
it('should keep nested unconditional css together', () => {
Expand All @@ -19,4 +10,43 @@ describe('css builder', () => {

expect(actual).toInclude('@media screen{._43475scu{color:red}._1yzygktf{font-size:20px}}');
});

it('generates the correct style prop for shadowed runtime identifiers', () => {
const actual = transform(`
import '@compiled/react';
const getBackgroundColor = (color) => color;
const color = baseColor;
<div css={{
backgroundColor: getBackgroundColor(customBackgroundColor),
color
}} />
`);

// Make sure color is used over customBackgroundColor
expect(actual).toIncludeMultiple(['{color:var(--_1ylxx6h)}', '"--_1ylxx6h": ix(color)']);

expect(actual).toMatchInlineSnapshot(`
"const _2 = \\"._syaz1aj3{color:var(--_1ylxx6h)}\\";
const _ = \\"._bfhk8ruw{background-color:var(--_agotg1)}\\";
const getBackgroundColor = (color) => color;
const color = baseColor;
<CC>
<CS>{[_, _2]}</CS>
{
<div
className={ax([\\"_bfhk8ruw _syaz1aj3\\"])}
style={{
\\"--_agotg1\\": ix(getBackgroundColor(customBackgroundColor)),
\\"--_1ylxx6h\\": ix(color),
}}
/>
}
</CC>;
"
`);
});
});
28 changes: 28 additions & 0 deletions packages/babel-plugin/src/__tests__/test-utils.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
import { transformSync } from '@babel/core';
import { format } from 'prettier';

import babelPlugin from '../babel-plugin';

export const transform = (code: string): string => {
const fileResult = transformSync(code, {
babelrc: false,
comments: false,
configFile: false,
plugins: [babelPlugin],
});

if (!fileResult || !fileResult.code) {
return '';
}

const { code: babelCode } = fileResult;
const ifIndex = babelCode.indexOf('if (');
// Remove the imports from the code, and the styled components display name
const snippet = babelCode
.substring(babelCode.indexOf('const'), ifIndex === -1 ? babelCode.length : ifIndex)
.trim();

return format(snippet, {
parser: 'babel',
});
};
4 changes: 3 additions & 1 deletion packages/babel-plugin/src/utils/evaluate-expression.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -220,7 +220,9 @@ const traverseFunction = (expression: t.Function, meta: Metadata) => {
const traverseCallExpression = (expression: t.CallExpression, meta: Metadata) => {
const callee = expression.callee;
let value: t.Node | undefined | null = undefined;
let updatedMeta: Metadata = meta;
// Make sure updatedMeta is a new object, so that when the ownPath is set, the meta does not get re-used incorrectly in
// later parts of the AST
let updatedMeta: Metadata = { ...meta };

/*
Basically flow is as follows:
Expand Down

0 comments on commit 48805ec

Please sign in to comment.