sync specific addition/removal of typings and do not retranspile on adding/removing packages, do it on first codechange

Author: christianalfoni

packages/app/src/app/overmind/effects/vscode/SandboxFsSync/index.ts

@@ -10,12 +10,16 @@ import {
   Sandbox,
   SandboxFs,
 } from '@codesandbox/common/lib/types';
-import { getAbsoluteDependencies } from '@codesandbox/common/lib/utils/dependencies';
+import {
+  fetchPackageJSON,
+  isAbsoluteVersion,
+} from '@codesandbox/common/lib/utils/dependencies';
 import { getGlobal } from '@codesandbox/common/lib/utils/global';
 import { protocolAndHost } from '@codesandbox/common/lib/utils/url-generator';
 import { json } from 'overmind';
-import { mkdir, rename, rmdir, unlink, writeFile } from './utils';
+
 import { WAIT_INITIAL_TYPINGS_MS } from '../constants';
+import { mkdir, rename, rmdir, unlink, writeFile } from './utils';
 
 const global = getGlobal() as Window & { BrowserFS: any };
 
@@ -36,8 +40,16 @@ type SandboxFsSyncOptions = {
 
 class SandboxFsSync {
   private options: SandboxFsSyncOptions;
-  private types: any = {};
-  private deps: { [path: string]: string } = {};
+  private types: {
+    [packageName: string]: {
+      [typeName: string]: string;
+    };
+  } = {};
+
+  private deps: { [path: string]: string } = {
+    '@types/jest': 'latest',
+  };
+
   private isDisposed = false;
   private currentSyncId = 0;
   private typesInfo: Promise<any>;
@@ -47,7 +59,10 @@ class SandboxFsSync {
   }
 
   public getTypes() {
-    return this.types;
+    return Object.keys(this.types).reduce(
+      (aggr, key) => Object.assign(aggr, this.types[key]),
+      {}
+    );
   }
 
   public dispose() {
@@ -180,23 +195,73 @@ class SandboxFsSync {
     try {
       this.currentSyncId = syncId;
 
+      this.typesInfo = await this.getTypesInfo();
       const syncDetails = await this.getDependencyTypingsSyncDetails();
 
       if (syncDetails) {
-        this.fetchDependencyTypings(
-          syncId,
-          syncDetails.packageJsonContent,
-          syncDetails.autoInstall
-        );
+        const newDeps = syncDetails.dependencies;
+        const { added, removed } = this.getDepsChanges(newDeps);
+
+        this.deps = newDeps;
+
+        added.forEach(dep => {
+          this.fetchDependencyTyping(syncId, dep, syncDetails.autoInstall);
+        });
+
+        if (removed.length) {
+          const removedTypings = {};
+
+          // We go through removed deps to figure out what typings packages
+          // has been removed, then delete the from our types as well
+          removed.forEach(removedDep => {
+            const typings = this.types[removedDep.name] || {};
+
+            Object.assign(removedTypings, typings);
+
+            delete this.types[removedDep.name];
+          });
+
+          this.send('types-remove', removedTypings);
+        }
       }
     } catch (error) {
       // eslint-disable-next-line
       console.error(error);
     }
   }
 
+  private getDepsChanges(newDeps) {
+    const added: Array<{ name: string; version: string }> = [];
+    const removed: Array<{ name: string; version: string }> = [];
+    const newDepsKeys = Object.keys(newDeps);
+    const currentDepsKeys = Object.keys(this.deps);
+
+    newDepsKeys.forEach(newDepKey => {
+      if (
+        !this.deps[newDepKey] ||
+        this.deps[newDepKey] !== newDeps[newDepKey]
+      ) {
+        added.push({
+          name: newDepKey,
+          version: newDeps[newDepKey],
+        });
+      }
+    });
+
+    currentDepsKeys.forEach(currentDepKey => {
+      if (currentDepKey !== '@types/jest' && !newDeps[currentDepKey]) {
+        removed.push({
+          name: currentDepKey,
+          version: this.deps[currentDepKey],
+        });
+      }
+    });
+
+    return { added, removed };
+  }
+
   private async getDependencyTypingsSyncDetails(): Promise<{
-    packageJsonContent: string;
+    dependencies: { [name: string]: string };
     autoInstall: boolean;
   }> {
     return new Promise((resolve, reject) => {
@@ -221,11 +286,21 @@ class SandboxFsSync {
                 browserFs.stat(
                   '/sandbox/tsconfig.json',
                   (tsConfigError, result) => {
-                    // If tsconfig exists we want to sync the types
-                    resolve({
-                      packageJsonContent: rv.toString(),
-                      autoInstall: Boolean(tsConfigError) || !result,
-                    });
+                    // If tsconfig exists we want to sync the typesp
+                    try {
+                      const packageJson = JSON.parse(rv.toString());
+                      resolve({
+                        dependencies: {
+                          ...packageJson.dependencies,
+                          ...packageJson.devDependencies,
+                        },
+                        autoInstall: Boolean(tsConfigError) || !result,
+                      });
+                    } catch (error) {
+                      reject(
+                        new Error('TYPINGS: Could not parse package.json')
+                      );
+                    }
                   }
                 );
               }
@@ -242,11 +317,11 @@ class SandboxFsSync {
   }
 
   private sendTypes() {
-    this.send('types-sync', this.types);
+    this.send('types-sync', this.getTypes());
   }
 
-  private sendType(type: { [path: string]: any }) {
-    this.send('type-sync', type);
+  private sendPackageTypes(types: { [path: string]: any }) {
+    this.send('package-types-sync', types);
   }
 
   /**
@@ -264,63 +339,62 @@ class SandboxFsSync {
     return this.typesInfo;
   }
 
-  private async fetchDependencyTypings(
+  private setAndSendPackageTypes(
+    name: string,
+    types: { [name: string]: string },
+    syncId: number
+  ) {
+    if (!this.isDisposed && this.currentSyncId === syncId) {
+      if (!this.types[name]) {
+        this.types[name] = {};
+      }
+
+      Object.assign(this.types[name], types);
+      this.sendPackageTypes(types);
+    }
+  }
+
+  private async fetchDependencyTyping(
     syncId: number,
-    packageJSON: string,
+    dep: { name: string; version: string },
     autoInstallTypes: boolean
   ) {
     try {
-      const { dependencies = {}, devDependencies = {} } = JSON.parse(
-        packageJSON
-      );
-
-      const totalDependencies = {
-        '@types/jest': 'latest',
-        ...dependencies,
-        ...devDependencies,
-      };
-
-      if (autoInstallTypes) {
-        const typeInfo = await this.getTypesInfo();
-        Object.keys(totalDependencies).forEach(async dep => {
-          if (
-            !dep.startsWith('@types/') &&
-            !totalDependencies[`@types/${dep}`] &&
-            typeInfo[dep]
-          ) {
-            totalDependencies[`@types/${dep}`] = typeInfo[dep].latest;
-          }
+      if (
+        autoInstallTypes &&
+        this.typesInfo[dep.name] &&
+        !dep.name.startsWith('@types/') &&
+        isAbsoluteVersion(dep.version)
+      ) {
+        const name = `@types/${dep.name}`;
+        fetchPackageJSON(name, dep.version).then(({ version }) => {
+          this.setAndSendPackageTypes(
+            dep.name,
+            {
+              [name]: version,
+            },
+            syncId
+          );
         });
       }
 
-      const absoluteDependencies = await getAbsoluteDependencies(
-        totalDependencies
-      );
-
-      Object.keys(absoluteDependencies).forEach(async depName => {
-        const depVersion = absoluteDependencies[depName];
-
-        try {
-          const fetchRequest = await fetch(
-            `${SERVICE_URL}/${depName}@${depVersion}.json`
-          );
+      try {
+        const fetchRequest = await fetch(
+          `${SERVICE_URL}/${dep.name}@${dep.version}.json`
+        );
 
-          if (!fetchRequest.ok) {
-            throw new Error('Fetch error');
-          }
+        if (!fetchRequest.ok) {
+          throw new Error('Fetch error');
+        }
 
-          const { files } = await fetchRequest.json();
+        const { files } = await fetchRequest.json();
 
-          if (!this.isDisposed && this.currentSyncId === syncId) {
-            Object.assign(this.types, files);
-            this.sendType(files);
-          }
-        } catch (e) {
-          if (process.env.NODE_ENV === 'development') {
-            console.warn('Trouble fetching types for ' + depName);
-          }
+        this.setAndSendPackageTypes(dep.name, files, syncId);
+      } catch (e) {
+        if (process.env.NODE_ENV === 'development') {
+          console.warn('Trouble fetching types for ' + dep.name);
         }
-      });
+      }
     } catch (e) {
       /* ignore */
     }

packages/app/src/app/overmind/effects/vscode/constants.ts

@@ -4,4 +4,4 @@ export const EXTENSIONS_LOCATION = process.env.VSCODE
 
 export const VIM_EXTENSION_ID = 'vscodevim.vim';
 
-export const WAIT_INITIAL_TYPINGS_MS = 2000;
+export const WAIT_INITIAL_TYPINGS_MS = 1000;

packages/app/src/app/overmind/effects/vscode/extensionHostWorker/common/fs.ts

@@ -101,8 +101,8 @@ export async function initializeBrowserFS({
     function touchFileSystem() {
       // This forces the file watchers to emit, which causes typescript to reload
       global.BrowserFS.BFSRequire('fs').rename(
-        '/sandbox/node_modules/@types',
-        '/sandbox/node_modules/@types',
+        '/sandbox/node_modules',
+        '/sandbox/node_modules',
         () => {}
       );
     }
@@ -125,11 +125,20 @@ export async function initializeBrowserFS({
               }
               break;
             }
-            case 'type-sync': {
+            case 'package-types-sync': {
               Object.assign(types, evt.data.$data);
               touchFileSystem();
               break;
             }
+            case 'types-remove': {
+              const deps = evt.data.$data;
+
+              Object.keys(deps).forEach(depKey => {
+                delete types[depKey];
+              });
+              touchFileSystem();
+              break;
+            }
             case 'sandbox-fs': {
               currentSandboxFs = evt.data.$data;
               if (isInitialSync) {

packages/app/src/app/overmind/namespaces/editor/actions.ts

@@ -45,8 +45,6 @@ export const addNpmDependency: AsyncAction<{
       version: newVersion,
       isDev: Boolean(isDev),
     });
-
-    actions.editor.internal.updatePreviewCode();
   }
 );
 
@@ -65,8 +63,6 @@ export const npmDependencyRemoved: AsyncAction<{
     moduleShortid: state.editor.currentPackageJSON.shortid,
     cbID: null,
   });
-
-  actions.editor.internal.updatePreviewCode();
 });
 
 export const sandboxChanged: AsyncAction<{ id: string }> = withLoadApp<{

packages/common/src/utils/dependencies.ts

@@ -18,7 +18,7 @@ async function fetchWithRetries(url: string) {
   throw err;
 }
 
-async function fetchPackageJSON(dep: string, version: string) {
+export async function fetchPackageJSON(dep: string, version: string) {
   try {
     return await fetchWithRetries(
       `https://unpkg.com/${dep}@${encodeURIComponent(version)}/package.json`
@@ -32,14 +32,16 @@ async function fetchPackageJSON(dep: string, version: string) {
   }
 }
 
-export async function getAbsoluteDependencies(dependencies: Object) {
-  const nonAbsoluteDependencies = Object.keys(dependencies).filter(dep => {
-    const version = dependencies[dep];
+export function isAbsoluteVersion(version: string) {
+  const isAbsolute = /^\d+\.\d+\.\d+$/.test(version);
 
-    const isAbsolute = /^\d+\.\d+\.\d+$/.test(version);
+  return isAbsolute || /\//.test(version);
+}
 
-    return !isAbsolute && !/\//.test(version);
-  });
+export async function getAbsoluteDependencies(dependencies: Object) {
+  const nonAbsoluteDependencies = Object.keys(dependencies).filter(
+    dep => !isAbsoluteVersion(dependencies[dep])
+  );
 
   const newDependencies = { ...dependencies };