fix(utils): logical and ternary expressions in code remover (fixes #1310

) (#1349)

* fix(utils): logical and ternary expressions in code remover (fixes #1310)

* feat(utils): support for SSR/browser checks

* fix(utils): do not delete alternate branch during if-optimization

.changeset/pretty-horses-behave.md

@@ -0,0 +1,5 @@
+---
+'@linaria/utils': patch
+---
+
+The code remover is trained to handle logical and ternary expressions. Fixes #1310.

.changeset/six-elephants-design.md

@@ -0,0 +1,5 @@
+---
+'@linaria/utils': patch
+---
+
+Browser-specific code can now be wrapped in `if (typeof window !== 'undefined') { /* will be deleted */ }`.

packages/utils/src/__tests__/__snapshots__/removeDangerousCode.test.ts.snap

@@ -9,6 +9,16 @@ export function setVersion(packageName, packageVersion) {
 }"
 `;
 
+exports[`removeDangerousCode should remove code under is-it-browser checks 1`] = `
+"let method;
+{
+  console.log('it is not browser');
+}
+export function testFn(arg) {
+  method(arg);
+}"
+`;
+
 exports[`removeDangerousCode should replace body of react component with null 1`] = `
 "export var Popup = /*#__PURE__*/function () {
   var Popup = function Popup() {
@@ -18,3 +28,45 @@ exports[`removeDangerousCode should replace body of react component with null 1`
   return Popup;
 }();"
 `;
+
+exports[`removeDangerousCode should simplify OR expression 1`] = `
+"export function getHostname(opts) {
+  return opts.hostname;
+}"
+`;
+
+exports[`removeDangerousCode should simplify if statement 1`] = `
+"{
+  console.log('it is not browser');
+}
+{
+  console.log('it is not browser');
+}
+{
+  console.log('it is not browser');
+}"
+`;
+
+exports[`removeDangerousCode should simplify ternary operator 1`] = `
+"export function getHostname(opts) {
+  return "localhost";
+}"
+`;
+
+exports[`removeDangerousCode should simplify ternary operator 2`] = `
+"export function getHostname(opts) {
+  return "localhost";
+}"
+`;
+
+exports[`removeDangerousCode should simplify ternary operator 3`] = `
+"export function getHostname(opts) {
+  return opts ? "localhost" : undefined;
+}"
+`;
+
+exports[`removeDangerousCode should simplify ternary operator 4`] = `
+"export function getHostname(opts) {
+  return opts ? undefined : "localhost";
+}"
+`;

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

@@ -2,11 +2,6 @@
 
 exports[`removeWithRelated should keep alive used import specifier 1`] = `"import { b } from './source';"`;
 
-exports[`removeWithRelated should keep logical expression 1`] = `
-"const c = 3;
-const res = false && c;"
-`;
-
 exports[`removeWithRelated should not delete params of functions 1`] = `
 "function test(arg) {
   return null;
@@ -28,6 +23,8 @@ const testArrow2 = () => {};
 export default function testDefaultFn() {}"
 `;
 
+exports[`removeWithRelated should optimize logical expression 1`] = `"const res = false;"`;
+
 exports[`removeWithRelated should remove node if it becomes invalid after removing its children 1`] = `""`;
 
 exports[`removeWithRelated should remove the whole import 1`] = `""`;

packages/utils/src/__tests__/removeDangerousCode.test.ts

@@ -64,4 +64,88 @@ describe('removeDangerousCode', () => {
 
     expect(result).toMatchSnapshot();
   });
+
+  it('should simplify ternary operator', () => {
+    expect(run`
+      export function getHostname(opts) {
+        return typeof location !== "undefined" ? location.hostname : "localhost";
+      }
+    `).toMatchSnapshot();
+
+    expect(run`
+      export function getHostname(opts) {
+        return location.hostname ? "location.hostname" : "localhost";
+      }
+    `).toMatchSnapshot();
+
+    expect(run`
+      export function getHostname(opts) {
+        return opts ? "localhost" : location.hostname;
+      }
+    `).toMatchSnapshot();
+
+    expect(run`
+      export function getHostname(opts) {
+        return opts ? location.hostname : "localhost";
+      }
+    `).toMatchSnapshot();
+  });
+
+  it('should simplify OR expression', () => {
+    const result = run`
+      export function getHostname(opts) {
+        return opts.hostname || location.hostname;
+      }
+    `;
+
+    expect(result).toMatchSnapshot();
+  });
+
+  it('should remove code under is-it-browser checks', () => {
+    const result = run`
+      let method;
+
+      if (typeof window !== 'undefined') {
+        console.log('it is browser');
+        method = window.fetch;
+      }
+
+      if (typeof window === 'undefined') {
+        console.log('it is not browser');
+        method = fetch;
+      }
+
+      export function testFn(arg) {
+        method(arg);
+      }
+    `;
+
+    expect(result).toMatchSnapshot();
+  });
+
+  it('should simplify if statement', () => {
+    const result = run`
+      if (typeof window !== 'undefined') {
+        console.log('it is browser');
+      }
+
+      if (typeof window === 'undefined') {
+        console.log('it is not browser');
+      }
+
+      if (typeof window === 'undefined') {
+        console.log('it is not browser');
+      } else {
+        console.log('it is browser');
+      }
+
+      if (typeof window !== 'undefined') {
+        console.log('it is browser');
+      } else {
+        console.log('it is not browser');
+      }
+    `;
+
+    expect(result).toMatchSnapshot();
+  });
 });

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

@@ -98,7 +98,7 @@ describe('removeWithRelated', () => {
     expect(code).toMatchSnapshot();
   });
 
-  it('should keep logical expression', () => {
+  it('should optimize logical expression', () => {
     const code = run`
       const a = 1;
       /* remove */const b = 2;

packages/utils/src/removeDangerousCode.ts

@@ -3,7 +3,7 @@ import type { Identifier, Program } from '@babel/types';
 
 import { nonType } from './findIdentifiers';
 import { isUnnecessaryReactCall } from './isUnnecessaryReactCall';
-import { removeWithRelated } from './scopeHelpers';
+import { applyAction, removeWithRelated } from './scopeHelpers';
 import JSXElementsRemover from './visitors/JSXElementsRemover';
 
 const isGlobal = (id: NodePath<Identifier>): boolean => {
@@ -16,28 +16,37 @@ const isGlobal = (id: NodePath<Identifier>): boolean => {
   return !scope.hasBinding(name) && scope.hasGlobal(name);
 };
 
+const ssrCheckFields = new Set([
+  'document',
+  'location',
+  'navigator',
+  'sessionStorage',
+  'localStorage',
+  'window',
+]);
+
 const forbiddenGlobals = new Set([
+  ...ssrCheckFields,
   '$RefreshReg$',
   'XMLHttpRequest',
   'clearImmediate',
   'clearInterval',
   'clearTimeout',
-  'document',
   'fetch',
-  'localStorage',
-  'location',
   'navigator',
-  'sessionStorage',
   'setImmediate',
   'setInterval',
   'setTimeout',
-  'window',
 ]);
 
 const isBrowserGlobal = (id: NodePath<Identifier>) => {
   return forbiddenGlobals.has(id.node.name) && isGlobal(id);
 };
 
+const isSSRCheckField = (id: NodePath<Identifier>) => {
+  return ssrCheckFields.has(id.node.name) && isGlobal(id);
+};
+
 const getPropertyName = (path: NodePath): string | null => {
   if (path.isIdentifier()) {
     return path.node.name;
@@ -133,6 +142,24 @@ export const removeDangerousCode = (programPath: NodePath<Program>) => {
           state.globals.push(p);
         }
       },
+
+      // Since we can use happy-dom, typical SSR checks may not work as expected.
+      // We need to detect them and replace with an "undefined" literal.
+      UnaryExpression(p) {
+        if (p.node.operator !== 'typeof') {
+          return;
+        }
+        const arg = p.get('argument');
+        if (!arg.isIdentifier() || !isSSRCheckField(arg)) {
+          return;
+        }
+
+        applyAction([
+          'replace',
+          p,
+          { type: 'StringLiteral', value: 'undefined' },
+        ]);
+      },
     },
     {
       globals: [] as NodePath<Identifier>[],

packages/utils/src/scopeHelpers.ts

@@ -279,6 +279,20 @@ export function findActionForNode(
     }
   }
 
+  if (parent.isConditionalExpression()) {
+    if (path.key === 'test') {
+      return ['replace', parent, parent.node.alternate];
+    }
+
+    if (path.key === 'consequent') {
+      return ['replace', path, { type: 'Identifier', name: 'undefined' }];
+    }
+
+    if (path.key === 'alternate') {
+      return ['replace', path, { type: 'Identifier', name: 'undefined' }];
+    }
+  }
+
   if (parent.isLogicalExpression({ operator: '&&' })) {
     return [
       'replace',
@@ -290,6 +304,14 @@ export function findActionForNode(
     ];
   }
 
+  if (parent.isLogicalExpression({ operator: '||' })) {
+    return [
+      'replace',
+      parent,
+      path.key === 'left' ? parent.node.right : parent.node.left,
+    ];
+  }
+
   if (parent.isObjectProperty()) {
     // let's check if it is a special case with Object.defineProperty
     const key = parent.get('key');
@@ -481,17 +503,87 @@ function removeUnreferenced(items: NodePath<Identifier | JSXIdentifier>[]) {
   return result;
 }
 
+function getNodeForValue(value: unknown): Node | undefined {
+  if (typeof value === 'string') {
+    return {
+      type: 'StringLiteral',
+      value,
+    };
+  }
+
+  if (typeof value === 'number') {
+    return {
+      type: 'NumericLiteral',
+      value,
+    };
+  }
+
+  if (typeof value === 'boolean') {
+    return {
+      type: 'BooleanLiteral',
+      value,
+    };
+  }
+
+  if (value === null) {
+    return {
+      type: 'NullLiteral',
+    };
+  }
+
+  if (value === undefined) {
+    return {
+      type: 'Identifier',
+      name: 'undefined',
+    };
+  }
+
+  return undefined;
+}
+
+function staticEvaluate(path: NodePath | null | undefined): void {
+  if (!path) return;
+  const evaluated = path.evaluate();
+  if (evaluated.confident) {
+    const node = getNodeForValue(evaluated.value);
+    if (node) {
+      applyAction(['replace', path, node]);
+      return;
+    }
+  }
+
+  if (path.isIfStatement()) {
+    const test = path.get('test');
+    if (!test.isBooleanLiteral()) {
+      return;
+    }
+
+    const { consequent, alternate } = path.node;
+    if (test.node.value) {
+      applyAction(['replace', path, consequent]);
+    } else if (alternate) {
+      applyAction(['replace', path, alternate]);
+    } else {
+      applyAction(['remove', path]);
+    }
+  }
+}
+
 function applyAction(action: ReplaceAction | RemoveAction) {
   mutate(action[1], (p) => {
     if (isRemoved(p)) return;
 
+    const parent = p.parentPath;
+
     if (action[0] === 'remove') {
       p.remove();
     }
 
     if (action[0] === 'replace') {
       p.replaceWith(action[2]);
     }
+
+    staticEvaluate(parent);
   });
 }