Put new packager full on (codesandbox#261)

* Put new packager full on

* Add some documentation

* Cache resolver paths

Author: CompuIves

src/app/components/sandbox/Preview/index.js

@@ -264,8 +264,6 @@ export default class Preview extends React.PureComponent<Props, State> {
         template,
         hasActions: !!runActionFromPreview,
         isModuleView: !isInProjectView,
-        // TODO remove this in 2 weeks
-        experimentalPackager: preferences.newPackagerExperiment,
       });
     }
   };

src/app/containers/Preferences/Experiments/Preferences.js

@@ -8,7 +8,7 @@ import preferencesActionCreators from 'app/store/preferences/actions';
 import {
   Container,
   PreferenceContainer,
-  PaddedPreference,
+  // PaddedPreference,
   Description,
 } from '../styles';
 
@@ -23,25 +23,22 @@ const mapDispatchToProps = dispatch => ({
 const mapStateToProps = state => ({
   preferences: preferencesSelector(state),
 });
+// eslint-disable-next-line no-unused-vars arrow-body-style
 const Preferences = ({ preferences, preferencesActions }: Props) => {
-  const bindValue = name => ({
-    value: preferences[name],
-    setValue: value =>
-      preferencesActions.setPreference({
-        [name]: value,
-      }),
-  });
+  // const bindValue = name => ({
+  //   value: preferences[name],
+  //   setValue: value =>
+  //     preferencesActions.setPreference({
+  //       [name]: value,
+  //     }),
+  // });
+
   return (
     <Container>
       <PreferenceContainer>
-        <PaddedPreference
-          title="New dependency bundler"
-          type="boolean"
-          {...bindValue('newPackagerExperiment')}
-        />
         <Description>
-          Use the new, faster, client side dependency bundler. Reload the
-          browser for changes to take effect.
+          We have no experiments running currently! Tune in later to find some
+          new goodies to test.
         </Description>
       </PreferenceContainer>
     </Container>

src/sandbox/compile.js

@@ -7,7 +7,6 @@ import _debug from 'app/utils/debug';
 import initializeErrorTransformers from './errors/transformers';
 import getPreset from './eval';
 import Manager from './eval/manager';
-import resolveDependency from './eval/loaders/dependency-resolver';
 
 import { resetScreen } from './status-screen';
 
@@ -53,16 +52,9 @@ function getIndexHtml(modules) {
   return '<div id="root"></div>';
 }
 
-async function updateManager(
-  sandboxId,
-  template,
-  managerModules,
-  experimentalPackager = false
-) {
+async function updateManager(sandboxId, template, managerModules) {
   if (!manager || manager.id !== sandboxId) {
-    manager = new Manager(sandboxId, managerModules, getPreset(template), {
-      experimentalPackager,
-    });
+    manager = new Manager(sandboxId, managerModules, getPreset(template));
   } else {
     await manager.updateData(managerModules);
   }
@@ -95,7 +87,6 @@ async function compile({
   hasActions,
   isModuleView = false,
   template,
-  experimentalPackager = false,
 }) {
   try {
     clearErrorTransformers();
@@ -116,21 +107,18 @@ async function compile({
     }));
 
     const [{ manifest, isNewCombination }] = await Promise.all([
-      loadDependencies(dependencies, experimentalPackager),
-      updateManager(sandboxId, template, managerModules, experimentalPackager),
+      loadDependencies(dependencies),
+      updateManager(sandboxId, template, managerModules),
     ]);
 
-    const { externals = {} } = manifest || {};
-    if (experimentalPackager) {
-      manager.setManifest(manifest);
-    } else {
-      manager.setExternals(externals);
-    }
-
+    // Just reset the whole packager if it's a new combination
     if (isNewCombination) {
-      manager.clearCompiledCache();
+      manager = null;
+      await updateManager(sandboxId, template, managerModules);
     }
 
+    manager.setManifest(manifest);
+
     const managerModulePathToTranspile = getModulePath(
       modules,
       directories,
@@ -149,9 +137,12 @@ async function compile({
     try {
       const children = document.body.children;
       // Do unmounting for react
-      if (externals['react-dom']) {
-        const reactDOM = resolveDependency('react-dom', externals);
+      if (manifest.dependencies.find(n => n.name === 'react-dom')) {
+        const reactDOMModule = manager.resolveModule('react-dom', '');
+        const reactDOM = manager.evaluateModule(reactDOMModule);
+
         reactDOM.unmountComponentAtNode(document.body);
+
         for (let i = 0; i < children.length; i += 1) {
           if (children[i].tagName === 'DIV') {
             reactDOM.unmountComponentAtNode(children[i]);

src/sandbox/eval/manager.js

@@ -57,29 +57,33 @@ export default class Manager {
   modules: ModuleObject;
 
   manifest: Manifest;
-  experimentalPackager: boolean;
 
-  constructor(
-    id: string,
-    modules: Array<Module>,
-    preset: Preset,
-    options: Object = {}
-  ) {
+  constructor(id: string, modules: Array<Module>, preset: Preset) {
     this.id = id;
     this.preset = preset;
     this.transpiledModules = {};
+    this.cachedPaths = {};
     modules.forEach(m => this.addModule(m));
 
-    this.experimentalPackager = options.experimentalPackager || false;
-
     if (process.env.NODE_ENV === 'development') {
       console.log(this);
     }
   }
 
-  setExternals(externals: Externals) {
-    this.externals = externals;
-  }
+  // Hoist these 2 functions to the top, since they get executed A LOT
+  isFile = (p: string) =>
+    !!this.transpiledModules[p] || !!getCombinedMetas()[p];
+
+  readFileSync = (p: string) => {
+    if (this.transpiledModules[p]) {
+      return this.transpiledModules[p].module.code;
+    }
+
+    const err = new Error('Could not find ' + p);
+    err.code = 'ENOENT';
+
+    throw err;
+  };
 
   setManifest(manifest: ?Manifest) {
     this.manifest = manifest || {
@@ -194,6 +198,7 @@ export default class Manager {
    * @param {*} entry
    */
   transpileModules(entry: Module) {
+    this.cachedPaths = {};
     const transpiledModule = this.getTranspiledModule(entry);
 
     transpiledModule.setIsEntry(true);
@@ -208,6 +213,20 @@ export default class Manager {
     return values(this.transpiledModules).map(t => t.module);
   }
 
+  /**
+   * The packager returns a list of dependencies that require a different path
+   * of their subdependencies.
+   *
+   * An example:
+   * if react requires lodash v3, and react-dom requires lodash v4. We add them
+   * both to the bundle, and rewrite paths for lodash v3 to `lodash/3.0.0/`. Then
+   * we specify that when react resolves `lodash` it should resolve `lodash/3.0.0`.
+   *
+   * @param {string} path
+   * @param {string} currentPath
+   * @returns
+   * @memberof Manager
+   */
   getAliasedDependencyPath(path: string, currentPath: string) {
     const isDependency = /^(\w|@\w)/.test(path);
 
@@ -237,29 +256,37 @@ export default class Manager {
     return path;
   }
 
+  // All paths are resolved at least twice: during transpilation and evaluation.
+  // We can improve performance by almost 2x in this scenario if we cache the lookups
+  cachedPaths: {
+    [path: string]: string,
+  } = {};
+
   resolveModule(
     path: string,
     currentPath: string,
     defaultExtensions: Array<string> = ['js', 'jsx', 'json']
   ): Module {
     const aliasedPath = this.getAliasedDependencyPath(path, currentPath);
     const shimmedPath = coreLibraries[aliasedPath] || aliasedPath;
+
+    const pathId = path + currentPath;
+    const cachedPath = this.cachedPaths[pathId];
     try {
-      const resolvedPath = resolve.sync(shimmedPath, {
-        filename: currentPath,
-        extensions: defaultExtensions.map(ext => '.' + ext),
-        isFile: p => !!this.transpiledModules[p] || !!getCombinedMetas()[p],
-        readFileSync: p => {
-          if (this.transpiledModules[p]) {
-            return this.transpiledModules[p].module.code;
-          }
-
-          const err = new Error('Could not find ' + p);
-          err.code = 'ENOENT';
-
-          throw err;
-        },
-      });
+      let resolvedPath;
+
+      if (cachedPath) {
+        resolvedPath = cachedPath;
+      } else {
+        resolvedPath = resolve.sync(shimmedPath, {
+          filename: currentPath,
+          extensions: defaultExtensions.map(ext => '.' + ext),
+          isFile: this.isFile,
+          readFileSync: this.readFileSync,
+        });
+
+        this.cachedPaths[pathId] = resolvedPath;
+      }
 
       if (NODE_LIBS.includes(resolvedPath)) {
         return {

src/sandbox/eval/transpiled-module.js

@@ -237,14 +237,6 @@ export default class TranspiledModule {
       // that include the source of another file by themselves, we need to
       // force transpilation to rebuild the file
       addTranspilationDependency: (depPath: string, options) => {
-        if (
-          /^(\w|@\w)/.test(depPath) &&
-          !depPath.includes('!') &&
-          !manager.experimentalPackager
-        ) {
-          return;
-        }
-
         const tModule = manager.resolveTranspiledModule(
           depPath,
           options && options.isAbsolute ? '/' : this.module.path
@@ -257,9 +249,6 @@ export default class TranspiledModule {
       },
       addDependency: (depPath: string, options) => {
         if (
-          (/^(\w|@\w)/.test(depPath) &&
-            !depPath.includes('!') &&
-            !manager.experimentalPackager) ||
           depPath.startsWith('babel-runtime') ||
           depPath.startsWith('codesandbox-api')
         ) {
@@ -288,14 +277,6 @@ export default class TranspiledModule {
         }
       },
       addDependenciesInDirectory: (folderPath: string, options) => {
-        if (
-          /^(\w|@\w)/.test(folderPath) &&
-          !folderPath.includes('!') &&
-          !manager.experimentalPackager
-        ) {
-          return;
-        }
-
         const tModules = manager.resolveTranspiledModulesInDirectory(
           folderPath,
           options && options.isAbsolute ? '/' : this.module.path
@@ -472,17 +453,10 @@ export default class TranspiledModule {
         if (/^(\w|@\w)/.test(aliasedPath) && !aliasedPath.includes('!')) {
           // So it must be a dependency
           if (
-            !manager.experimentalPackager ||
             aliasedPath.startsWith('babel-runtime') ||
             aliasedPath.startsWith('codesandbox-api')
           )
-            // TODO remove this hack for not-experimental packager
-            return resolveDependency(
-              path === 'vue' && !manager.experimentalPackager
-                ? path
-                : aliasedPath,
-              manager.externals
-            );
+            return resolveDependency(aliasedPath, manager.externals);
         }
 
         const requiredTranspiledModule = manager.resolveTranspiledModule(