prevent mutation of cached ASTs (#1153)

src/Module.js

@@ -2,7 +2,7 @@ import { parse } from 'acorn/src/index.js';
 import MagicString from 'magic-string';
 import { locate } from 'locate-character';
 import { timeStart, timeEnd } from './utils/flushTime.js';
-import { assign, blank, deepClone, keys } from './utils/object.js';
+import { assign, blank, keys } from './utils/object.js';
 import { basename, extname } from './utils/path.js';
 import makeLegalIdentifier from './utils/makeLegalIdentifier.js';
 import getCodeFrame from './utils/getCodeFrame.js';
@@ -12,6 +12,7 @@ import relativeId from './utils/relativeId.js';
 import { SyntheticNamespaceDeclaration } from './Declaration.js';
 import extractNames from './ast/utils/extractNames.js';
 import enhance from './ast/enhance.js';
+import clone from './ast/clone.js';
 import ModuleScope from './ast/scopes/ModuleScope.js';
 
 function tryParse ( code, comments, acornOptions, id ) {
@@ -41,8 +42,15 @@ export default class Module {
 
 		timeStart( 'ast' );
 
-		this.ast = ast || tryParse( code, this.comments, bundle.acornOptions, id ); // TODO what happens to comments if AST is provided?
-		this.astClone = deepClone( this.ast );
+		if ( ast ) {
+			// prevent mutating the provided AST, as it may be reused on
+			// subsequent incremental rebuilds
+			this.ast = clone( ast );
+			this.astClone = ast;
+		} else {
+			this.ast = tryParse( code, this.comments, bundle.acornOptions, id ); // TODO what happens to comments if AST is provided?
+			this.astClone = clone( this.ast );
+		}
 
 		timeEnd( 'ast' );
 

src/ast/clone.js

@@ -0,0 +1,17 @@
+export default function clone ( node ) {
+	if ( !node ) return node;
+	if ( typeof node !== 'object' ) return node;
+
+	if ( Array.isArray( node ) ) {
+		const cloned = new Array( node.length );
+		for ( let i = 0; i < node.length; i += 1 ) cloned[i] = clone( node[i] );
+		return cloned;
+	}
+
+	const cloned = {};
+	for ( const key in node ) {
+		cloned[ key ] = clone( node[ key ] );
+	}
+
+	return cloned;
+}

src/utils/object.js

@@ -17,24 +17,3 @@ export function assign ( target, ...sources ) {
 
 	return target;
 }
-
-const isArray = Array.isArray;
-
-// used for cloning ASTs. Not for use with cyclical structures!
-export function deepClone ( obj ) {
-	if ( !obj ) return obj;
-	if ( typeof obj !== 'object' ) return obj;
-
-	if ( isArray( obj ) ) {
-		const clone = new Array( obj.length );
-		for ( let i = 0; i < obj.length; i += 1 ) clone[i] = deepClone( obj[i] );
-		return clone;
-	}
-
-	const clone = {};
-	for ( const key in obj ) {
-		clone[ key ] = deepClone( obj[ key ] );
-	}
-
-	return clone;
-}

test/test.js

@@ -684,6 +684,35 @@ describe( 'rollup', function () {
 				assert.deepEqual( asts.foo, acorn.parse( modules.foo, { sourceType: 'module' }) );
 			});
 		});
+
+		it( 'recovers from errors', () => {
+			modules.entry = `import foo from 'foo'; import bar from 'bar'; export default foo + bar;`;
+
+			return rollup.rollup({
+				entry: 'entry',
+				plugins: [ plugin ]
+			}).then( cache => {
+				modules.foo = `var 42 = nope;`;
+
+				return rollup.rollup({
+					entry: 'entry',
+					plugins: [ plugin ],
+					cache
+				}).catch( err => {
+					return cache;
+				});
+			}).then( cache => {
+				modules.foo = `export default 42;`;
+
+				return rollup.rollup({
+					entry: 'entry',
+					plugins: [ plugin ],
+					cache
+				}).then( bundle => {
+					assert.equal( executeBundle( bundle ), 63 );
+				});
+			});
+		});
 	});
 
 	describe( 'hooks', () => {