feat(styled): adds first pass at not passing down invalid html attrib…

…utes to the styled node

package.json

@@ -30,6 +30,7 @@
     "postversion": "echo 'make sure to push \"git push && git push --tags\"'"
   },
   "dependencies": {
+    "@emotion/is-prop-valid": "^0.8.6",
     "stylis": "^3.5.4",
     "typescript": "^3.7.3"
   },

src/styled/__tests__/index.test.tsx

@@ -33,4 +33,17 @@ describe('styled component', () => {
 
     expect(getByText('hello world')).toHaveCompiledCss('font-size', '12px');
   });
+
+  it('should not pass down invalid html attributes to the node', () => {
+    const size = '12px';
+    const StyledDiv = styled.div<{ fonty: string }>`
+      font-size: ${props => props.fonty};
+    `;
+
+    const { getByText, debug } = render(<StyledDiv fonty={size}>hello world</StyledDiv>);
+
+    debug();
+
+    expect(getByText('hello world').getAttribute('fonty')).toBe(null);
+  });
 });

src/ts-transformer/class-names/visitors/visit-class-names-jsx-element.tsx

@@ -68,7 +68,7 @@ export const visitClassNamesJsxElement = (
         // create new object literal using original object literal properties
         styleObjectLiteral.properties.concat(
           cssVariables.map(cssVar =>
-            ts.createPropertyAssignment(ts.createStringLiteral(cssVar.name), cssVar.identifier)
+            ts.createPropertyAssignment(ts.createStringLiteral(cssVar.name), cssVar.expression)
           )
         )
       );

src/ts-transformer/css-prop/visitors/visit-jsx-element-with-css-prop.tsx

@@ -128,7 +128,7 @@ export const visitJsxElementWithCssProp = (
                 cssVariables.map(cssVariable =>
                   ts.createPropertyAssignment(
                     ts.createStringLiteral(cssVariable.name),
-                    cssVariable.identifier
+                    cssVariable.expression
                   )
                 )
               )

src/ts-transformer/styled-component/__tests__/index.test.tsx

@@ -27,6 +27,18 @@ describe('styled component transformer', () => {
     );
   });
 
+  it('should not pass down invalid html attributes to the node', () => {
+    const actual = transformer.transform(`
+      import { styled } from '${pkg.name}';
+      const ListItem = styled.div({
+        fontSize: props => props.textSize,
+      });
+    `);
+
+    expect(actual).toInclude('({ textSize, ...props }) =>');
+    expect(actual).toInclude('"--textSize-test-css-variable": textSize');
+  });
+
   xit('should compose using a previously created component', () => {
     const actual = transformer.transform(`
       import { styled } from '${pkg.name}';

src/ts-transformer/styled-component/visitors/visit-styled-component.tsx

@@ -1,9 +1,11 @@
 import ts from 'typescript';
+import isPropValid from '@emotion/is-prop-valid';
 import { createJsxElement } from '../../utils/create-jsx-element';
 import { objectLiteralToCssString } from '../../utils/object-literal-to-css';
 import { templateLiteralToCss } from '../../utils/template-literal-to-css';
 import { VariableDeclarations } from '../../types';
 import { joinToJsxExpression } from '../../utils/expression-operators';
+import { getIdentifierText } from '../../utils/ast-node';
 
 const getTagName = (node: ts.CallExpression | ts.TaggedTemplateExpression): string => {
   if (ts.isCallExpression(node) && ts.isPropertyAccessExpression(node.expression)) {
@@ -17,6 +19,14 @@ const getTagName = (node: ts.CallExpression | ts.TaggedTemplateExpression): stri
   throw new Error('tag should have been here');
 };
 
+const getPropertyAccessName = (propertyAccess?: string): string => {
+  if (!propertyAccess) {
+    return '';
+  }
+
+  return propertyAccess.indexOf('.') > 0 ? propertyAccess.split('.')[1] : propertyAccess;
+};
+
 const getObjectLiteralOrTemplateLiteral = (
   node: ts.CallExpression | ts.TaggedTemplateExpression
 ): ts.ObjectLiteralExpression | ts.TemplateExpression | ts.NoSubstitutionTemplateLiteral => {
@@ -44,12 +54,35 @@ export const visitStyledComponent = (
     ? objectLiteralToCssString(dataToTransform, collectedDeclarations, context)
     : templateLiteralToCss(dataToTransform, collectedDeclarations, context);
 
+  const propsToDestructure = result.cssVariables
+    .map(({ expression }) => {
+      if (ts.isIdentifier(expression)) {
+        // referencing an identifier straight e.g. props.fontSize
+        const propName = getPropertyAccessName(expression.text);
+        if (!isPropValid(propName)) {
+          return propName;
+        }
+      } else {
+        // is an expression e.g. props.fontSize + 'px'
+      }
+    })
+    .filter(Boolean) as string[];
+
   const newElement = createJsxElement(
     tagName,
     {
       ...result,
       originalNode: node,
-      styleProperties: [ts.createSpreadAssignment(ts.createIdentifier('props.style'))],
+      styleFactory: props => [
+        ts.createSpreadAssignment(ts.createIdentifier('props.style')),
+        ...props.map(prop => {
+          const propName = getPropertyAccessName(getIdentifierText(prop.initializer));
+          if (propsToDestructure.includes(propName)) {
+            prop.initializer = ts.createIdentifier(propName);
+          }
+          return prop;
+        }),
+      ],
       classNameFactory: className =>
         joinToJsxExpression(className, ts.createIdentifier('props.className'), {
           conditional: true,
@@ -67,7 +100,21 @@ export const visitStyledComponent = (
         undefined,
         undefined,
         undefined,
-        ts.createIdentifier('props'),
+        propsToDestructure.length
+          ? // We want to destructure props so it doesn't contain any invalid html attributes.
+            ts.createObjectBindingPattern([
+              ...propsToDestructure.map(prop =>
+                ts.createBindingElement(undefined, undefined, ts.createIdentifier(prop), undefined)
+              ),
+              ts.createBindingElement(
+                ts.createToken(ts.SyntaxKind.DotDotDotToken),
+                undefined,
+                ts.createIdentifier('props'),
+                undefined
+              ),
+            ])
+          : // They're all valid so we don't need to destructure.
+            ts.createIdentifier('props'),
         undefined,
         undefined,
         undefined

src/ts-transformer/types.tsx

@@ -9,7 +9,7 @@ export interface CssVariableExpressions {
   /**
    * Identifier of the css variable, as in the literal ts.Identifier.
    */
-  identifier: ts.Identifier | ts.BinaryExpression;
+  expression: ts.Identifier | ts.BinaryExpression;
 }
 
 export interface VariableDeclarations {

src/ts-transformer/utils/create-jsx-element.tsx

@@ -8,7 +8,9 @@ interface JsxElementOpts {
   css: string;
   cssVariables: CssVariableExpressions[];
   originalNode: ts.Node;
-  styleProperties?: (ts.PropertyAssignment | ts.SpreadAssignment)[];
+  styleFactory?: (
+    props: ts.PropertyAssignment[]
+  ) => (ts.PropertyAssignment | ts.SpreadAssignment)[];
   classNameFactory?: (node: ts.StringLiteral) => ts.StringLiteral | ts.JsxExpression;
   jsxAttributes?: (ts.JsxAttribute | ts.JsxSpreadAttribute)[];
   selector?: string;
@@ -109,6 +111,13 @@ export const createJsxElement = (tagNode: string, opts: JsxElementOpts, original
   );
 
   if (opts.cssVariables.length) {
+    const styleProps = opts.cssVariables.map(variable => {
+      return ts.createPropertyAssignment(
+        ts.createStringLiteral(variable.name),
+        variable.expression
+      );
+    });
+
     // TODO: we could pass this into jsx opening element
     const elementNodeAttributes = getJsxNodeAttributes(elementNode);
     (elementNodeAttributes.properties as any).push(
@@ -117,14 +126,7 @@ export const createJsxElement = (tagNode: string, opts: JsxElementOpts, original
         ts.createJsxExpression(
           undefined,
           ts.createObjectLiteral(
-            (opts.styleProperties || []).concat(
-              opts.cssVariables.map(variable => {
-                return ts.createPropertyAssignment(
-                  ts.createStringLiteral(variable.name),
-                  variable.identifier
-                );
-              })
-            ),
+            opts.styleFactory ? opts.styleFactory(styleProps) : styleProps,
             false
           )
         )

src/ts-transformer/utils/extract-css-var-from-arrow-function.tsx

@@ -24,5 +24,5 @@ export const extractCssVarFromArrowFunction = (
 
   ts.visitEachChild(node, visitor, context);
 
-  return { identifier, name };
+  return { expression: identifier, name };
 };

src/ts-transformer/utils/object-literal-to-css.tsx

@@ -96,7 +96,7 @@ export const objectLiteralToCssString = (
       value = `var(${cssVariable})`;
       cssVariables.push({
         name: cssVariable,
-        identifier: getAssignmentIdentifier(prop),
+        expression: getAssignmentIdentifier(prop),
       });
     } else if (ts.isPropertyAssignment(prop) && ts.isObjectLiteralExpression(prop.initializer)) {
       key = kebabCase((prop.name as ts.Identifier).text);

src/ts-transformer/utils/template-literal-to-css.tsx

@@ -74,7 +74,7 @@ export const templateLiteralToCss = (
         const result = extractSuffix(span.literal.text);
         cssVariables.push({
           name: variableName,
-          identifier: result.suffix
+          expression: result.suffix
             ? // Join left + right if suffix is defined
               joinToBinaryExpression(span.expression, ts.createStringLiteral(result.suffix))
             : // Else just return the expression we found
@@ -94,14 +94,14 @@ export const templateLiteralToCss = (
       const result = extractCssVarFromArrowFunction(span.expression, context);
       cssVariables.push({
         name: result.name,
-        identifier: extractedSuffix.suffix
+        expression: extractedSuffix.suffix
           ? // Join left + right if suffix is defined
             joinToBinaryExpression(
-              result.identifier,
+              result.expression,
               ts.createStringLiteral(extractedSuffix.suffix)
             )
           : // Else just return the expression we found
-            result.identifier,
+            result.expression,
       });
       css += `var(${result.name})${extractedSuffix.rest}`;
     } else if (ts.isCallExpression(span.expression)) {

yarn.lock

@@ -895,10 +895,20 @@
   dependencies:
     "@emotion/memoize" "0.7.3"
 
+"@emotion/is-prop-valid@^0.8.6":
+  version "0.8.6"
+  resolved "https://registry.yarnpkg.com/@emotion/is-prop-valid/-/is-prop-valid-0.8.6.tgz#4757646f0a58e9dec614c47c838e7147d88c263c"
+  dependencies:
+    "@emotion/memoize" "0.7.4"
+
 "@emotion/memoize@0.7.3":
   version "0.7.3"
   resolved "https://registry.yarnpkg.com/@emotion/memoize/-/memoize-0.7.3.tgz#5b6b1c11d6a6dddf1f2fc996f74cf3b219644d78"
 
+"@emotion/memoize@0.7.4":
+  version "0.7.4"
+  resolved "https://registry.yarnpkg.com/@emotion/memoize/-/memoize-0.7.4.tgz#19bf0f5af19149111c40d98bb0cf82119f5d9eeb"
+
 "@emotion/serialize@^0.11.12", "@emotion/serialize@^0.11.14":
   version "0.11.14"
   resolved "https://registry.yarnpkg.com/@emotion/serialize/-/serialize-0.11.14.tgz#56a6d8d04d837cc5b0126788b2134c51353c6488"