refactor(@angular-devkit/schematics): improve performance of move() (#12857)

FrozenPandaz · vikerman · commit 13c057a8541a · 2018-11-07T14:25:27.000-08:00
diff --git a/packages/angular_devkit/schematics/src/rules/move.ts b/packages/angular_devkit/schematics/src/rules/move.ts
@@ -23,9 +23,17 @@ export function move(from: string, to?: string): Rule {
     return noop;
   }
 
-  return tree => tree.visit(path => {
-    if (path.startsWith(fromPath)) {
-      tree.rename(path, toPath + '/' + path.substr(fromPath.length));
+  return tree => {
+    if (tree.exists(fromPath)) {
+      // fromPath is a file
+      tree.rename(fromPath, toPath);
+    } else {
+      // fromPath is a directory
+      tree.getDir(fromPath).visit(path => {
+        tree.rename(path, toPath + '/' + path.substr(fromPath.length));
+      });
     }
-  });
+
+    return tree;
+  };
 }
diff --git a/packages/angular_devkit/schematics/src/rules/move_spec.ts b/packages/angular_devkit/schematics/src/rules/move_spec.ts
@@ -49,6 +49,40 @@ describe('move', () => {
       .then(done, done.fail);
   });
 
+  it('works on moving a directory into a subdirectory of itself', done => {
+    const tree = new HostTree();
+    tree.create('a/b/file1', 'hello world');
+    tree.create('a/b/file2', 'hello world');
+    tree.create('a/c/file3', 'hello world');
+
+    callRule(move('a/b', 'a/b/c'), observableOf(tree), context)
+      .toPromise()
+      .then(result => {
+        expect(result.exists('a/b/c/file1')).toBe(true);
+        expect(result.exists('a/b/c/file2')).toBe(true);
+        expect(result.exists('a/c/file3')).toBe(true);
+      })
+      .then(done, done.fail);
+  });
+
+  it('works on moving a directory into a parent of itself', done => {
+    const tree = new HostTree();
+    tree.create('a/b/file1', 'hello world');
+    tree.create('a/b/file2', 'hello world');
+    tree.create('a/c/file3', 'hello world');
+
+    callRule(move('a/b', 'a'), observableOf(tree), context)
+      .toPromise()
+      .then(result => {
+        expect(result.exists('file1')).toBe(false);
+        expect(result.exists('file2')).toBe(false);
+        expect(result.exists('a/file1')).toBe(true);
+        expect(result.exists('a/file2')).toBe(true);
+        expect(result.exists('a/c/file3')).toBe(true);
+      })
+      .then(done, done.fail);
+  });
+
   it('becomes a noop with identical from and to', done => {
     const tree = new HostTree();
     tree.create('a/b/file1', 'hello world');
diff --git a/packages/angular_devkit/schematics/src/tree/host-tree.ts b/packages/angular_devkit/schematics/src/tree/host-tree.ts
@@ -76,23 +76,25 @@ export class HostDirEntry implements DirEntry {
   }
 
   visit(visitor: FileVisitor): void {
-    function _recurse(entry: DirEntry) {
-      entry.subfiles.forEach(path => {
-        visitor(join(entry.path, path), entry.file(path));
-      });
-      entry.subdirs.forEach(path => {
-        _recurse(entry.dir(path));
-      });
-    }
-
     try {
-      _recurse(this);
+      this.getSubfilesRecursively().forEach(file => visitor(file.path, file));
     } catch (e) {
       if (e !== FileVisitorCancelToken) {
         throw e;
       }
     }
   }
+
+  private getSubfilesRecursively() {
+    function _recurse(entry: DirEntry): FileEntry[] {
+      return entry.subdirs.reduce((files, subdir) => [
+        ...files,
+        ..._recurse(entry.dir(subdir)),
+      ], entry.subfiles.map(subfile => entry.file(subfile) as FileEntry));
+    }
+
+    return _recurse(this);
+  }
 }
 
 
@@ -315,12 +317,9 @@ export class HostTree implements Tree {
     return maybeCache;
   }
   visit(visitor: FileVisitor): void {
-    const allFiles: [Path, FileEntry | null | undefined][] = [];
     this.root.visit((path, entry) => {
-      allFiles.push([path, entry]);
+      visitor(path, entry);
     });
-
-    allFiles.forEach(([path, entry]) => visitor(path, entry));
   }
 
   // Change content of host files.
diff --git a/packages/angular_devkit/schematics/src/tree/virtual.ts b/packages/angular_devkit/schematics/src/tree/virtual.ts
@@ -84,24 +84,26 @@ export class VirtualDirEntry implements DirEntry {
     return this._tree.get(join(this._path, name));
   }
 
-  visit(visitor: FileVisitor) {
-    function _recurse(entry: VirtualDirEntry) {
-      entry.subfiles.forEach(path => {
-        visitor(join(entry._path, path), entry.file(path));
-      });
-      entry.subdirs.forEach(path => {
-        _recurse(entry.dir(path) as VirtualDirEntry);
-      });
-    }
-
+  visit(visitor: FileVisitor): void {
     try {
-      _recurse(this);
+      this.getSubfilesRecursively().forEach(file => visitor(file.path, file));
     } catch (e) {
       if (e !== FileVisitorCancelToken) {
         throw e;
       }
     }
   }
+
+  private getSubfilesRecursively() {
+    function _recurse(entry: DirEntry): FileEntry[] {
+      return entry.subdirs.reduce((files, subdir) => [
+        ...files,
+        ..._recurse(entry.dir(subdir)),
+      ], entry.subfiles.map(subfile => entry.file(subfile) as FileEntry));
+    }
+
+    return _recurse(this);
+  }
 }
 
 /**
