Fix/replace identifiers (BuilderIO#961)

* dedupe @babel/types * augment babel types * update yarn * comment * update yarn lock * add missing path for jsx-runtime * remove redundant type workaround * improve replace-identifiers * tweak * refactor, pass hook key * improve logging * debug * remove patch * fix fiddle * revert tsconfig changes * revert to yarn 3.2 * run yarn * add some missing deps for fiddle * undo @babel/types dedupe * remove
jesse-forked · Dec 22, 2022 · 5456136 · 5456136
1 parent 3717ef3
commit 5456136
Show file tree

Hide file tree

Showing 10 changed files with 112 additions and 35 deletions.
diff --git a/.gitignore b/.gitignore
@@ -18,6 +18,7 @@ packages/core/mitosis-build
 .yarn/*
 !/.yarn/patches
 !/.yarn/versions
+!/.yarn/releases
 
 # sveltekit tmp files
 **/package/__package_types_tmp__/

diff --git a/.yarnrc.yml b/.yarnrc.yml
@@ -1,10 +1,9 @@
 enableGlobalCache: true
 
-# `pnp` causes tons of type errors in `core`
-nodeLinker: node-modules
-
 nmHoistingLimits: workspaces
 
+nodeLinker: node-modules
+
 plugins:
   - path: .yarn/plugins/@yarnpkg/plugin-workspace-tools.cjs
     spec: "@yarnpkg/plugin-workspace-tools"

diff --git a/packages/core/src/generators/vue/helpers.ts b/packages/core/src/generators/vue/helpers.ts
@@ -166,14 +166,9 @@ export const processBinding = ({
           }
         },
       }),
-      (x) => {
-        return pipe(
-          x,
-          (code) => processRefs(code, json, options),
-          (code) => prefixMethodsWithThis(code, json, options),
-          (code) => (preserveGetter === false ? stripGetter(code) : code),
-        );
-      },
+      (code) => processRefs(code, json, options),
+      (code) => prefixMethodsWithThis(code, json, options),
+      (code) => (preserveGetter === false ? stripGetter(code) : code),
     );
   } catch (e) {
     console.log('could not process bindings in ', { code });

diff --git a/packages/core/src/helpers/__snapshots__/replace-identifiers.test.ts.snap b/packages/core/src/helpers/__snapshots__/replace-identifiers.test.ts.snap
@@ -32,3 +32,16 @@ exports[`replaceIdentifiers > Check #3 1`] = `"!state.useLazyLoading() || state.
 exports[`replaceIdentifiers > Check #4 1`] = `"state.name = 'PatrickJS onInit' + this.hi"`;
 
 exports[`replaceIdentifiers > Check #5 1`] = `"lowerCaseName()"`;
+
+exports[`replaceIdentifiers > Check #6 1`] = `
+"const x = {
+  foo: bar,
+  test: 123
+};
+const foo.value = x.foo;
+const y = {
+  l: x.foo,
+  m: foo.value
+};
+const bar = foo.value"
+`;
diff --git a/packages/core/src/helpers/plugins/process-code.ts b/packages/core/src/helpers/plugins/process-code.ts
@@ -1,12 +1,16 @@
 import { Plugin } from '../../types/plugins';
-import { MitosisComponent } from '../../types/mitosis-component';
+import { extendedHook, MitosisComponent } from '../../types/mitosis-component';
 import { MitosisNode } from '../../types/mitosis-node';
 import { checkIsDefined } from '../nullable';
 import { traverseNodes } from '../traverse-nodes';
 
 type CodeType = 'hooks' | 'hooks-deps' | 'bindings' | 'properties' | 'state';
 
-type CodeProcessor = (codeType: CodeType) => (code: string) => string;
+declare function codeProcessor(
+  codeType: CodeType,
+): (code: string, hookType?: keyof MitosisComponent['hooks']) => string;
+
+type CodeProcessor = typeof codeProcessor;
 
 /**
  * Process code in bindings and properties of a node
@@ -18,7 +22,7 @@ const preProcessBlockCode = ({
   json: MitosisNode;
   codeProcessor: CodeProcessor;
 }) => {
-  const propertiesProcessor = codeProcessor('properties');
+  // const propertiesProcessor = codeProcessor('properties');
   // for (const key in json.properties) {
   //   const value = json.properties[key];
   //   if (key !== '_text' && value) {
@@ -43,7 +47,12 @@ export const CODE_PROCESSOR_PLUGIN =
   () => ({
     json: {
       post: (json: MitosisComponent) => {
-        const processHookCode = codeProcessor('hooks');
+        function processHook(key: keyof typeof json.hooks, hook: extendedHook) {
+          hook.code = codeProcessor('hooks')(hook.code, key);
+          if (hook.deps) {
+            hook.deps = codeProcessor('hooks-deps')(hook.deps);
+          }
+        }
 
         /**
          * process code in hooks
@@ -52,17 +61,13 @@ export const CODE_PROCESSOR_PLUGIN =
           const typedKey = key as keyof typeof json.hooks;
           const hooks = json.hooks[typedKey];
 
-          if (checkIsDefined(hooks) && Array.isArray(hooks)) {
-            for (const hook of hooks) {
-              hook.code = processHookCode(hook.code);
-              if (hook.deps) {
-                hook.deps = codeProcessor('hooks-deps')(hook.deps);
+          if (checkIsDefined(hooks)) {
+            if (Array.isArray(hooks)) {
+              for (const hook of hooks) {
+                processHook(typedKey, hook);
               }
-            }
-          } else if (checkIsDefined(hooks)) {
-            hooks.code = processHookCode(hooks.code);
-            if (hooks.deps) {
-              hooks.deps = codeProcessor('hooks-deps')(hooks.deps);
+            } else {
+              processHook(typedKey, hooks);
             }
           }
         }

diff --git a/packages/core/src/helpers/replace-identifiers.test.ts b/packages/core/src/helpers/replace-identifiers.test.ts
@@ -44,6 +44,28 @@ const TEST_SPECS: Spec[] = [
     from: 'state',
     to: (name) => (name === 'children' ? '$$slots.default' : name),
   },
+  {
+    code: `
+    const x = {
+      foo: bar,
+      test: 123,
+    }
+
+    const foo = x.foo;
+
+    const y = {
+      l: x.foo,
+      m: foo
+    }
+
+    const bar = foo;
+    `,
+    from: ['foo', 'test'],
+    to: (name) => {
+      console.log({ name });
+      return `${name}.value`;
+    },
+  },
 ];
 
 describe('replaceIdentifiers', () => {

diff --git a/packages/core/src/helpers/replace-identifiers.ts b/packages/core/src/helpers/replace-identifiers.ts
@@ -3,6 +3,18 @@ import generate from '@babel/generator';
 import { pipe } from 'fp-ts/lib/function';
 import { babelTransformExpression } from './babel-transform';
 
+/**
+ * Type hack.
+ *
+ * We want to augment the `BaseNode` interface to include a `_builder_meta` property but couldn't get
+ * `yarn patch-package` to cooperate with us. So we're doing it this way.
+ */
+type AllowMeta<T = types.Node> = T & {
+  _builder_meta?: {
+    newlyGenerated: boolean;
+  };
+};
+
 type ReplaceArgs = {
   code: string;
   from: string | string[];
@@ -17,18 +29,19 @@ const getToParam = (
   path: babel.NodePath<types.Identifier | types.MemberExpression | types.OptionalMemberExpression>,
 ): string => {
   if (types.isMemberExpression(path.node) || types.isOptionalMemberExpression(path.node)) {
-    // if simple member expression e.g. `props.foo`, returns `foo`
     if (types.isIdentifier(path.node.property)) {
-      const newLocal = path.node.property.name;
-      return newLocal;
+      // if simple member expression e.g. `props.foo`, returns `foo`
+      const propertyName = path.node.property.name;
+      return propertyName;
     } else {
       // if nested member expression e.g. `props.foo.bar.baz`, returns `foo.bar.baz`
       const x = generate(path.node.property).code;
       return x;
     }
   } else {
     // if naked identifier e.g. `foo`, returns `foo`
-    return path.node.name;
+    const nodeName = path.node.name;
+    return nodeName;
   }
 };
 
@@ -39,7 +52,10 @@ const _replaceIdentifiers = (
   const memberExpressionObject = types.isIdentifier(path.node) ? path.node : path.node.object;
   const normalizedFrom = Array.isArray(from) ? from : [from];
 
-  if (!types.isIdentifier(memberExpressionObject)) {
+  if (
+    !types.isIdentifier(memberExpressionObject) ||
+    (path.node as AllowMeta)?._builder_meta?.newlyGenerated
+  ) {
     return;
   }
 
@@ -91,16 +107,16 @@ const _replaceIdentifiers = (
           if (generate(path.node).code === generate(newMemberExpression).code) {
             return;
           }
-
+          (newMemberExpression as AllowMeta)._builder_meta = { newlyGenerated: true };
           path.replaceWith(newMemberExpression);
         } catch (err) {
-          console.error('Could not replace', path.node, 'with', to);
+          console.debug('Could not replace', path.node, 'with', to.toString());
           // throw err;
         }
       }
     } else {
       if (types.isIdentifier(path.node)) {
-        console.error(`Could not replace Identifier '${from.toString()}' with nothing.`);
+        console.debug(`Could not replace Identifier '${from.toString()}' with nothing.`);
       } else {
         // if we're looking at a member expression, e.g. `props.foo` and no `to` was provided, then we want to strip out
         // the identifier and end up with `foo`. So we replace the member expression with just its `property` value.
@@ -128,13 +144,15 @@ export const replaceIdentifiers = ({ code, from, to }: ReplaceArgs) => {
             !types.isOptionalMemberExpression(path.parent) &&
             // function declaration identifiers shouldn't be transformed
             !types.isFunctionDeclaration(path.parent)
+            // variable declaration identifiers shouldn't be transformed
+            // !(types.isVariableDeclarator(path.parent) && path.parent.id === path.node)
           ) {
             _replaceIdentifiers(path, { from, to });
           }
         },
       }),
       // merely running `babel.transform` will add spaces around the code, even if we don't end up replacing anything.
-      // This is why we need to trim the output.
+      // we have some other code downstream that cannot have untrimmed spaces, so we need to trim the output.
       (code) => code.trim(),
     );
   } catch (err) {

diff --git a/packages/core/tsconfig.json b/packages/core/tsconfig.json
@@ -13,7 +13,7 @@
     "target": "ES5",
     "baseUrl": ".",
     "paths": {
-      "@builder.io/mitosis": ["./src"]
+      "@builder.io/mitosis": ["./src"],
     },
     "jsxImportSource": "@builder.io/mitosis",
     "types": ["vitest/globals"]

diff --git a/packages/fiddle/package.json b/packages/fiddle/package.json
@@ -21,16 +21,19 @@
     "lz-string": "^1.4.4",
     "mobx": "^6.6.0",
     "mobx-react-lite": "^3.4.0",
+    "module": "^1.2.5",
     "monaco-editor": "^0.34.0",
     "monaco-editor-webpack-plugin": "^7.0.1",
     "next": "^12.2.5",
     "next-transpile-modules": "^9.0.0",
     "null-loader": "^4.0.1",
+    "pnpapi": "^0.0.0",
     "prettier": "^2.7.1",
     "prettier-plugin-svelte": "^2.8.0",
     "react": "^18.2.0",
     "react-dom": "^18.2.0",
     "react-use": "^17.4.0",
+    "semver": "^7.3.8",
     "svelte": "^3.53.1",
     "tsconfig-paths-webpack-plugin": "^3.5.2",
     "vertx": "^0.0.1-security",

diff --git a/yarn.lock b/yarn.lock
@@ -2623,17 +2623,20 @@ __metadata:
     lz-string: ^1.4.4
     mobx: ^6.6.0
     mobx-react-lite: ^3.4.0
+    module: ^1.2.5
     monaco-editor: ^0.34.0
     monaco-editor-webpack-plugin: ^7.0.1
     next: ^12.2.5
     next-transpile-modules: ^9.0.0
     null-loader: ^4.0.1
+    pnpapi: ^0.0.0
     prettier: ^2.7.1
     prettier-plugin-svelte: ^2.8.0
     raw-loader: ^4.0.2
     react: ^18.2.0
     react-dom: ^18.2.0
     react-use: ^17.4.0
+    semver: ^7.3.8
     svelte: ^3.53.1
     tsconfig-paths-webpack-plugin: ^3.5.2
     vertx: ^0.0.1-security
@@ -19645,6 +19648,13 @@ __metadata:
   languageName: node
   linkType: hard
 
+"pnpapi@npm:^0.0.0":
+  version: 0.0.0
+  resolution: "pnpapi@npm:0.0.0"
+  checksum: 9b8ce695cb5dcd904f2602a5e943d5808a58818e5c5dcf1c9b6dde3610b0f9070b9bc6758621c39d2cc954a1305d6256d925f176e17f00cc30779a6350d7a9f7
+  languageName: node
+  linkType: hard
+
 "popper.js@npm:1.16.1-lts":
   version: 1.16.1-lts
   resolution: "popper.js@npm:1.16.1-lts"
@@ -22010,6 +22020,17 @@ __metadata:
   languageName: node
   linkType: hard
 
+"semver@npm:^7.3.8":
+  version: 7.3.8
+  resolution: "semver@npm:7.3.8"
+  dependencies:
+    lru-cache: ^6.0.0
+  bin:
+    semver: bin/semver.js
+  checksum: ba9c7cbbf2b7884696523450a61fee1a09930d888b7a8d7579025ad93d459b2d1949ee5bbfeb188b2be5f4ac163544c5e98491ad6152df34154feebc2cc337c1
+  languageName: node
+  linkType: hard
+
 "send@npm:0.17.2":
   version: 0.17.2
   resolution: "send@npm:0.17.2"