Prevent circular import crash

src/DependencyGraph.spec.ts

@@ -142,10 +142,15 @@ describe('DependencyGraph', () => {
             expect(mock.callCount).to.equal(1);
         });
 
-        it('emits immediately when configured to do so', () => {
-            let mock = sinon.mock();
-            graph.onchange('a', mock, true);
-            expect(mock.callCount).to.equal(1);
+        it('does not cause infinite loop on circular dependency', () => {
+            //direct
+            graph.addOrReplace('a', ['b']);
+            graph.addOrReplace('b', ['a']);
+
+            //indirect
+            graph.addOrReplace('c', ['d']);
+            graph.addOrReplace('d', ['e']);
+            graph.addOrReplace('e', ['c']);
         });
     });
 

src/DependencyGraph.ts

@@ -9,7 +9,10 @@ export class DependencyGraph {
      */
     public nodes = {} as Record<string, Node>;
 
-    private onchangeEmitter = new EventEmitter();
+    /**
+     * An internal event emitter for when keys have changed.
+     */
+    private onchangeEmitter = new EventEmitter<string, DependencyChangedEvent>();
 
     /**
      * Add a node to the graph.
@@ -26,7 +29,7 @@ export class DependencyGraph {
         //create a new dependency node
         let node = new Node(key, dependencies, this);
         this.nodes[key] = node;
-        this.onchangeEmitter.emit(key, key);
+        this.emit(key, { sourceKey: key, notifiedKeys: new Set() });
     }
 
     /**
@@ -70,25 +73,26 @@ export class DependencyGraph {
      */
     public remove(key: string) {
         delete this.nodes[key];
-        this.onchangeEmitter.emit(key, key);
+        this.emit(key, { sourceKey: key, notifiedKeys: new Set() });
     }
 
     /**
      * Emit event that this item has changed
      */
-    public emit(key: string) {
-        this.onchangeEmitter.emit(key, key);
+    public emit(key: string, event: DependencyChangedEvent) {
+        //prevent infinite event loops by skipping already-notified keys
+        if (!event.notifiedKeys.has(key)) {
+            event.notifiedKeys.add(key);
+            this.onchangeEmitter.emit(key, event);
+        }
     }
 
     /**
      * Listen for any changes to dependencies with the given key.
      * @param emitImmediately if true, the handler will be called once immediately.
      */
-    public onchange(key: string, handler: (key) => void, emitImmediately = false) {
+    public onchange(key: string, handler: (event: DependencyChangedEvent) => void) {
         this.onchangeEmitter.on(key, handler);
-        if (emitImmediately) {
-            this.onchangeEmitter.emit(key, key);
-        }
         return () => {
             this.onchangeEmitter.off(key, handler);
         };
@@ -103,6 +107,17 @@ export class DependencyGraph {
     }
 }
 
+export interface DependencyChangedEvent {
+    /**
+     * The key that was the initiator of this event. Child keys will emit this same event object, but this key will remain the same
+     */
+    sourceKey: string;
+    /**
+     * A set of keys that have already been notified of this change. Used to prevent circular reference notification cycles
+     */
+    notifiedKeys: Set<string>;
+}
+
 export class Node {
     public constructor(
         public key: string,
@@ -113,9 +128,9 @@ export class Node {
             this.subscriptions = [];
         }
         for (let dependency of this.dependencies) {
-            let sub = this.graph.onchange(dependency, () => {
+            let sub = this.graph.onchange(dependency, (event) => {
                 //notify the graph that we changed since one of our dependencies changed
-                this.graph.emit(this.key);
+                this.graph.emit(this.key, event);
             });
 
             this.subscriptions.push(sub);

src/Scope.ts

@@ -16,6 +16,7 @@ import { URI } from 'vscode-uri';
 import { LogLevel } from './Logger';
 import { isBrsFile, isClassStatement, isFunctionStatement, isFunctionType, isXmlFile, isCustomType, isClassMethodStatement } from './astUtils/reflection';
 import type { BrsFile } from './files/BrsFile';
+import type { DependencyChangedEvent } from './DependencyGraph';
 
 /**
  * A class to keep track of all declarations within a given scope (like source scope, component scope)
@@ -32,8 +33,10 @@ export class Scope {
 
         //anytime a dependency for this scope changes, we need to be revalidated
         this.programHandles.push(
-            this.program.dependencyGraph.onchange(this.dependencyGraphKey, this.onDependenciesChanged.bind(this), true)
+            this.program.dependencyGraph.onchange(this.dependencyGraphKey, this.onDependenciesChanged.bind(this))
         );
+        //invalidate immediately since this is a new scope
+        this.invalidate();
     }
 
     /**
@@ -119,8 +122,8 @@ export class Scope {
      */
     protected diagnostics = [] as BsDiagnostic[];
 
-    protected onDependenciesChanged(key: string) {
-        this.logDebug('invalidated because dependency graph said [', key, '] changed');
+    protected onDependenciesChanged(event: DependencyChangedEvent) {
+        this.logDebug('invalidated because dependency graph said [', event.sourceKey, '] changed');
         this.invalidate();
     }
 

src/files/XmlFile.ts

@@ -489,7 +489,7 @@ export class XmlFile {
 
         if (this.needsTranspiled || extraImportScripts.length > 0) {
             //temporarily add the missing imports as script tags
-            const originalScripts = this.ast.component.scripts;
+            const originalScripts = this.ast.component?.scripts ?? [];
             this.ast.component.scripts = [
                 ...originalScripts,
                 ...extraImportScripts