fix(babel): avoid unnecessary babel transformations (#1019)

* fix(babel): apply only necessary transformations (#1018)

* fix(shaker): remove ObjectMethod if its body is removed

.changeset/purple-stingrays-carry.md

@@ -0,0 +1,6 @@
+---
+'@linaria/babel-preset': patch
+'@linaria/utils': patch
+---
+
+Shaker tried to keep alive object methods even if their body was removed (fixes #1018)

.changeset/shaggy-hats-lay.md

@@ -0,0 +1,6 @@
+---
+'@linaria/babel-preset': minor
+'@linaria/testkit': minor
+---
+
+Instead of just replacing tags with their runtime versions, `transform` mistakenly applied all babel transformations. (fixes #1018)

packages/babel/src/transform-stages/1-prepare-for-eval.ts

@@ -62,9 +62,10 @@ function runPreevalStage(
     configFile: false,
   });
 
-  const tmp = { ...transformConfig, filename };
-
-  const result = babel.transformFromAstSync(file, code, tmp);
+  const result = babel.transformFromAstSync(file, code, {
+    ...transformConfig,
+    filename,
+  });
 
   if (!result || !result.ast?.program) {
     throw new Error('Babel transform failed');

packages/babel/src/transform-stages/3-prepare-for-runtime.ts

@@ -1,6 +1,6 @@
 import * as babel from '@babel/core';
 
-import { loadBabelOptions } from '@linaria/utils';
+import { buildOptions, loadBabelOptions } from '@linaria/utils';
 
 import type { Options, ValueCache } from '../types';
 
@@ -25,32 +25,32 @@ export default function prepareForRuntime(
 
   const file = cachedParseSync(code, babelOptions);
 
-  const result = babel.transformFromAstSync(file, code, {
-    ...(babelOptions?.rootMode ? { rootMode: babelOptions.rootMode } : null),
-    cwd: babelConfig.cwd,
-    filename: babelConfig.filename ?? options.filename,
-    presets: [
-      ...(babelOptions?.presets ?? []),
-      ...(babelConfig?.presets ?? []),
-    ],
-    plugins: [
-      [
-        require.resolve('../plugins/collector'),
-        {
-          ...pluginOptions,
-          values: valueCache,
-        },
-      ],
-      ...(babelOptions?.plugins ?? []),
-      ...(babelConfig?.plugins ?? []),
+  const transformPlugins: babel.PluginItem[] = [
+    [
+      require.resolve('../plugins/collector'),
+      {
+        ...pluginOptions,
+        values: valueCache,
+      },
     ],
-    babelrc: false,
-    configFile: false,
+  ];
+
+  const transformConfig = buildOptions({
+    envName: 'linaria',
+    plugins: transformPlugins,
     sourceMaps: true,
-    sourceFileName: options.filename,
+    sourceFileName: babelConfig.filename ?? options.filename,
     inputSourceMap: options.inputSourceMap,
     root: options.root,
     ast: true,
+    babelrc: false,
+    configFile: false,
+  });
+
+  const result = babel.transformFromAstSync(file, code, {
+    ...transformConfig,
+    cwd: babelConfig.cwd,
+    filename: babelConfig.filename ?? options.filename,
   });
 
   if (!result || !result.ast?.program) {

packages/testkit/src/__snapshots__/babel.test.ts.snap

@@ -444,19 +444,43 @@ Dependencies: NA
 exports[`strategy shaker does not strip istanbul coverage sequences 1`] = `
 "function cov_2dr9r1nt95() {
   var path = \\"/home/user/project/file.js\\";
-  var hash = \\"91161e7e5b00b52dc483815fb73cd0b6fb94a785\\";
+  var hash = \\"36a1bd3d459d3717fbdf9bd0068161503f8053a5\\";
   var global = new Function(\\"return this\\")();
   var gcv = \\"__coverage__\\";
   var coverageData = {
     path: \\"/home/user/project/file.js\\",
-    statementMap: {},
+    statementMap: {
+      \\"0\\": {
+        start: {
+          line: 2,
+          column: 10
+        },
+        end: {
+          line: 2,
+          column: 12
+        }
+      },
+      \\"1\\": {
+        start: {
+          line: 4,
+          column: 26
+        },
+        end: {
+          line: 6,
+          column: 1
+        }
+      }
+    },
     fnMap: {},
     branchMap: {},
-    s: {},
+    s: {
+      \\"0\\": 0,
+      \\"1\\": 0
+    },
     f: {},
     b: {},
     _coverageSchema: \\"1a1c01bbd47fc00a2c39e90264f33305004495a9\\",
-    hash: \\"91161e7e5b00b52dc483815fb73cd0b6fb94a785\\"
+    hash: \\"36a1bd3d459d3717fbdf9bd0068161503f8053a5\\"
   };
   var coverage = global[gcv] || (global[gcv] = {});
 
@@ -475,14 +499,14 @@ exports[`strategy shaker does not strip istanbul coverage sequences 1`] = `
 }
 
 cov_2dr9r1nt95();
-export const titleClass = \\"titleClass_tow9xsn\\";"
+export const titleClass = \\"titleClass_t13jq05\\";"
 `;
 
 exports[`strategy shaker does not strip istanbul coverage sequences 2`] = `
 
 CSS:
 
-.titleClass_tow9xsn {
+.titleClass_t13jq05 {
   height: 42px;
 }
 
@@ -828,12 +852,9 @@ Dependencies: NA
 
 exports[`strategy shaker evaluates typescript enums 1`] = `
 "import { styled } from '@linaria/react';
-var Colors;
-
-(function (Colors) {
-  Colors[\\"BLUE\\"] = \\"#27509A\\";
-})(Colors || (Colors = {}));
-
+enum Colors {
+  BLUE = '#27509A',
+}
 export const Title = /*#__PURE__*/styled('h1')({
   name: \\"Title\\",
   class: \\"Title_t9vkbjs\\"
@@ -977,12 +998,7 @@ Dependencies: NA
 
 `;
 
-exports[`strategy shaker handles css template literal in JSX element 1`] = `
-"/*#__PURE__*/
-React.createElement(Title, {
-  class: \\"Title_twgemqq\\"
-});"
-`;
+exports[`strategy shaker handles css template literal in JSX element 1`] = `"<Title class={\\"Title_twgemqq\\"} />;"`;
 
 exports[`strategy shaker handles css template literal in JSX element 2`] = `
 
@@ -1600,7 +1616,7 @@ export function Something() {
       \\"tjtmpns-0\\": [_exp3()]
     }
   });
-  return /*#__PURE__*/React.createElement(Title, null);
+  return <Title />;
 }"
 `;
 
@@ -1952,7 +1968,7 @@ Dependencies: ./__fixtures__/reexports
 exports[`strategy shaker should wrap memoized components 1`] = `
 "import React from 'react';
 import { styled } from '@linaria/react';
-const MyComponent = /*#__PURE__*/React.memo(() => /*#__PURE__*/React.createElement(\\"div\\", null));
+const MyComponent = React.memo(() => <div />);
 
 const _exp = /*#__PURE__*/() => MyComponent;
 
@@ -1979,22 +1995,22 @@ exports[`strategy shaker simplifies react components 1`] = `
 import { styled } from '@linaria/react';
 import constant from './broken-dependency';
 
-const FuncComponent = props => /*#__PURE__*/React.createElement(\\"div\\", null, props.children + constant);
+const FuncComponent = props => <div>{props.children + constant}</div>;
 
 class ClassComponent extends React.PureComponent {
   method() {
     return constant;
   }
 
   render() {
-    return /*#__PURE__*/React.createElement(\\"div\\", null, props.children + constant);
+    return <div>{props.children + constant}</div>;
   }
 
 }
 
 const ComplexFunctionComponent = props => {
   if (import.meta.env.PROD) {
-    return /*#__PURE__*/React.createElement(\\"div\\", null, props.children + constant);
+    return <div>{props.children + constant}</div>;
   }
 
   return null;
@@ -2250,6 +2266,10 @@ Dependencies: NA
 
 exports[`strategy shaker transpiles styled template literal with TS component 1`] = `
 "import { styled } from '@linaria/react';
+type Props = {
+  className?: string;
+  children?: React.ReactNode;
+};
 export const Title = /*#__PURE__*/styled(() => {})({
   name: \\"Title\\",
   class: \\"Title_t9vkbjs\\"
@@ -2338,8 +2358,12 @@ Dependencies: NA
 
 exports[`strategy shaker transpiles with typed fn as interpolated value 1`] = `
 "import { styled } from '@linaria/react';
+type Props = {
+  className?: string;
+  children?: React.ReactNode;
+};
 
-const _exp = /*#__PURE__*/() => props => props.className;
+const _exp = /*#__PURE__*/() => (props: Props) => props.className;
 
 export const Title = /*#__PURE__*/styled('div')({
   name: \\"Title\\",

packages/testkit/src/babel.test.ts

@@ -1856,7 +1856,7 @@ describe('strategy shaker', () => {
   });
 
   it('does not strip istanbul coverage sequences', async () => {
-    const { code, metadata } = await transform(
+    const withIstanbul = await babel.transformAsync(
       dedent`
       import { css } from './custom-css-tag';
       const a = 42;
@@ -1865,35 +1865,39 @@ describe('strategy shaker', () => {
         height: ${'${a}'}px;
       \`;
       `,
-      [
-        evaluator,
-        {
-          tagResolver: (source, tag) => {
-            if (source === './custom-css-tag' && tag === 'css') {
-              return require.resolve('@linaria/core/processors/css');
-            }
-
-            return null;
-          },
-        },
-        'js',
-        {
-          cwd: '/home/user/project',
-          filename: 'file.js',
-          plugins: [
-            [
-              // eslint-disable-next-line import/no-extraneous-dependencies
-              require('babel-plugin-istanbul').default({
-                ...babel,
-                assertVersion: () => {},
-              }),
-              { cwd: '/home/user/project' },
-            ],
+      {
+        cwd: '/home/user/project',
+        filename: 'file.js',
+        plugins: [
+          [
+            // eslint-disable-next-line import/no-extraneous-dependencies
+            require('babel-plugin-istanbul').default({
+              ...babel,
+              assertVersion: () => {},
+            }),
+            { cwd: '/home/user/project' },
           ],
-        },
-      ]
+        ],
+      }
     );
 
+    if (!withIstanbul?.code) {
+      return;
+    }
+
+    const { code, metadata } = await transform(withIstanbul.code, [
+      evaluator,
+      {
+        tagResolver: (source, tag) => {
+          if (source === './custom-css-tag' && tag === 'css') {
+            return require.resolve('@linaria/core/processors/css');
+          }
+
+          return null;
+        },
+      },
+    ]);
+
     expect(code).toMatchSnapshot();
     expect(metadata).toMatchSnapshot();
   });

packages/utils/src/scopeHelpers.ts

@@ -139,7 +139,10 @@ export function findParentForDelete(path: NodePath): NodePath | null {
     return findParentForDelete(parent);
   }
 
-  if (parent.isFunctionExpression({ body: path.node })) {
+  if (
+    parent.isFunctionExpression({ body: path.node }) ||
+    parent.isObjectMethod()
+  ) {
     return findParentForDelete(parent);
   }