fix(shaker): get rid of "expected node to be of a type" errors (#1090)

.changeset/forty-crabs-itch.md

@@ -0,0 +1,8 @@
+---
+'@linaria/babel-preset': patch
+'@linaria/testkit': patch
+'@linaria/utils': patch
+'@linaria/shaker': patch
+---
+
+Get rid of "expected node to be of a type" errors

packages/babel/src/plugins/preeval.ts

@@ -8,9 +8,10 @@ import type { Identifier } from '@babel/types';
 import { createCustomDebug } from '@linaria/logger';
 import type { StrictOptions } from '@linaria/utils';
 import {
+  JSXElementsRemover,
   getFileIdx,
   isUnnecessaryReactCall,
-  JSXElementsRemover,
+  nonType,
   removeWithRelated,
 } from '@linaria/utils';
 
@@ -24,6 +25,10 @@ export type PreevalOptions = Pick<
 >;
 
 const isGlobal = (id: NodePath<Identifier>): boolean => {
+  if (!nonType(id)) {
+    return false;
+  }
+
   const { scope } = id;
   const { name } = id.node;
   return !scope.hasBinding(name) && scope.hasGlobal(name);
@@ -141,6 +146,8 @@ export default function preeval(
               }
 
               removeWithRelated([p]);
+
+              return;
             }
 
             if (state.windowScoped.has(p.node.name)) {

packages/testkit/package.json

@@ -17,6 +17,8 @@
     "typescript": "^4.7.4"
   },
   "devDependencies": {
+    "@babel/plugin-syntax-jsx": "^7.18.6",
+    "@babel/plugin-syntax-typescript": "^7.18.6",
     "@babel/plugin-transform-modules-commonjs": "^7.18.2",
     "@babel/runtime": "^7.18.3",
     "@babel/types": "^7.18.9",

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

@@ -6,8 +6,6 @@ exports[`preeval should keep getGlobal but remove window-related code 1`] = `
     return globalThis;
   }
 
-  if (typeof window !== \\"undefined\\") {}
-
   if (typeof global !== \\"undefined\\") {
     return global;
   }
@@ -20,4 +18,10 @@ exports[`preeval should keep getGlobal but remove window-related code 1`] = `
 }"
 `;
 
+exports[`preeval should not remove "location" in types only because it looks like a global variable 1`] = `
+"interface IProps {
+  fn: (location: string) => void;
+}"
+`;
+
 exports[`preeval should remove usages of window scoped identifiers 1`] = `""`;

packages/testkit/src/preeval.test.ts

@@ -6,14 +6,15 @@ import dedent from 'dedent';
 import { preeval } from '@linaria/babel-preset';
 
 const run = (code: TemplateStringsArray) => {
-  const filename = join(__dirname, 'source.js');
+  const filename = join(__dirname, 'source.ts');
   const formattedCode = dedent(code);
 
   const transformed = transformSync(formattedCode, {
     babelrc: false,
     configFile: false,
     filename,
     plugins: [
+      '@babel/plugin-syntax-typescript',
       [
         preeval,
         {
@@ -72,4 +73,14 @@ describe('preeval', () => {
 
     expect(code).toMatchSnapshot();
   });
+
+  it('should not remove "location" in types only because it looks like a global variable', () => {
+    const { code } = run`
+      interface IProps {
+        fn: (location: string) => void;
+      }
+    `;
+
+    expect(code).toMatchSnapshot();
+  });
 });

packages/testkit/src/utils/__snapshots__/removeWithRelated.test.ts.snap

@@ -15,6 +15,8 @@ exports[`removeWithRelated should remove export 1`] = `
 export { b };"
 `;
 
+exports[`removeWithRelated should remove node if it becomes invalid after removing its children 1`] = `""`;
+
 exports[`removeWithRelated should remove try/catch block 1`] = `"const a = 1;"`;
 
 exports[`removeWithRelated should shake try/catch 1`] = `""`;

packages/testkit/src/utils/removeWithRelated.test.ts

@@ -118,4 +118,14 @@ describe('removeWithRelated', () => {
 
     expect(code).toMatchSnapshot();
   });
+
+  it('should remove node if it becomes invalid after removing its children', () => {
+    const code = run`
+      /* remove */const mode = "DEV";
+
+      export { mode };
+    `;
+
+    expect(code).toMatchSnapshot();
+  });
 });

packages/utils/src/findIdentifiers.ts

@@ -32,7 +32,8 @@ export function nonType(path: NodePath): boolean {
       p.isTSTypeReference() ||
       p.isTSTypeQuery() ||
       p.isFlowType() ||
-      p.isFlowDeclaration()
+      p.isFlowDeclaration() ||
+      p.isTSInterfaceDeclaration()
   );
 }
 

packages/utils/src/index.ts

@@ -5,7 +5,7 @@ export {
 } from './asyncResolveFallback';
 export { default as collectExportsAndImports } from './collectExportsAndImports';
 export * from './collectExportsAndImports';
-export { default as findIdentifiers } from './findIdentifiers';
+export { default as findIdentifiers, nonType } from './findIdentifiers';
 export { default as getFileIdx } from './getFileIdx';
 export { default as isExports } from './isExports';
 export { default as isNotNull } from './isNotNull';

packages/utils/src/scopeHelpers.ts

@@ -2,13 +2,36 @@
 /* eslint @typescript-eslint/no-use-before-define: ["error", { "functions": false }] */
 
 import type { Binding, NodePath } from '@babel/traverse';
-import type { Identifier, JSXIdentifier, Program } from '@babel/types';
+import type {
+  Node,
+  Identifier,
+  JSXIdentifier,
+  Program,
+  FieldOptions,
+} from '@babel/types';
+import { NODE_FIELDS } from '@babel/types';
 
 import findIdentifiers, { nonType } from './findIdentifiers';
 import { getScope } from './getScope';
 import isNotNull from './isNotNull';
 import isRemoved from './isRemoved';
 
+function validateField(
+  node: Node,
+  key: string,
+  val: unknown,
+  field: FieldOptions
+) {
+  if (!(field != null && field.validate)) return true;
+  if (field.optional && val == null) return true;
+  try {
+    field.validate(node, key, val);
+    return true;
+  } catch {
+    return false;
+  }
+}
+
 function getBinding(path: NodePath<Identifier | JSXIdentifier>) {
   const binding = getScope(path).getBinding(path.node.name);
   if (!binding) {
@@ -199,22 +222,6 @@ export function findParentForDelete(path: NodePath): NodePath | null {
     return findParentForDelete(parent);
   }
 
-  if (parent.isExportDefaultDeclaration()) {
-    return findParentForDelete(parent);
-  }
-
-  if (parent.isTryStatement()) {
-    return findParentForDelete(parent);
-  }
-
-  if (parent.isExportSpecifier()) {
-    return findParentForDelete(parent);
-  }
-
-  if (parent.isConditionalExpression()) {
-    return findParentForDelete(parent);
-  }
-
   for (const key of ['body', 'declarations', 'specifiers']) {
     if (path.listKey === key && typeof path.key === 'number') {
       const list = parent.get(key) as NodePath[];
@@ -224,6 +231,18 @@ export function findParentForDelete(path: NodePath): NodePath | null {
     }
   }
 
+  if (parent.isTryStatement()) {
+    return findParentForDelete(parent);
+  }
+
+  if (!path.listKey) {
+    const field = NODE_FIELDS[parent.type][path.key];
+    if (!validateField(parent.node, path.key as string, null, field)) {
+      // The parent node isn't valid without this field, so we should remove it also.
+      return findParentForDelete(parent);
+    }
+  }
+
   for (const key of [
     'argument',
     'block',