Deno: Modules are executed every time they're imported.

[ry]

They should be executed only once - their exports cached.

Node behavior

~/src/deno> cat test.js
require("./foo");
require("./foo");
~/src/deno> cat foo.js
console.log("HELLO");
~/src/deno> node test.js
HELLO
~/src/deno>

Current deno behavior

~/src/deno> cat test.ts
import "./foo.ts";
import "./foo.ts";
~/src/deno> cat foo.ts
console.log("HELLO");
~/src/deno> ./out/debug/deno test.ts
HELLO
HELLO
~/src/deno>

[satyarohith]

Should we also warn them about duplication of imports?

[kitsonk]

No, duplication of imports can often happen, and be intentional, for example:

import * as foo from "./foo.ts";
import { bar } from "./foo.ts";

If it is valid TypeScript/JavaScript (which it is) there should be no warning.

[ry]

@kitsonk Sure that's valid and there should be no warning - it's about how many times the top-level function is executed.
Try this:

~/src/deno> cat test.html
<script type="module">
  import { printHi } from './foo.js'
  import * as foo from './foo.js'

  foo.printHi();
  printHi();
</script>
~/src/deno> cat foo.js
export function printHi () {
  console.log("hi");
}

console.log("top-level");

When I open test.html in my browser I get

top-level
hi
hi

[ry]

@kitsonk What do you think of this fix?

From 5061bf873f542a80a33db1a7c280d25bc4ca7a7f Mon Sep 17 00:00:00 2001
From: Ryan Dahl <[email protected]>
Date: Thu, 23 Aug 2018 22:29:07 -0400
Subject: [PATCH] Add failing test and potential fix for #587

---
 js/compiler.ts                               | 32 ++++++++++++++++------------
 tests/modules_should_be_executed_once.ts     |  2 ++
 tests/modules_should_be_executed_once.ts.out |  1 +
 tests/subdir/print_hello_on_import.ts        |  1 +
 4 files changed, 22 insertions(+), 14 deletions(-)
 create mode 100644 tests/modules_should_be_executed_once.ts
 create mode 100644 tests/modules_should_be_executed_once.ts.out
 create mode 100644 tests/subdir/print_hello_on_import.ts

diff --git a/js/compiler.ts b/js/compiler.ts
index fcbfc7c..c45a939 100644
--- a/js/compiler.ts
+++ b/js/compiler.ts
@@ -61,6 +61,7 @@ export class ModuleMetaData {
   public readonly exports = {};
   public scriptSnapshot?: ts.IScriptSnapshot;
   public scriptVersion = "";
+  public hasRun = false

   constructor(
     public readonly fileName: string,
@@ -401,21 +402,24 @@ export class DenoCompiler implements ts.LanguageServiceHost {
   ): ModuleMetaData {
     this._log("run", { moduleSpecifier, containingFile });
     const moduleMetaData = this.resolveModule(moduleSpecifier, containingFile);
-    const fileName = moduleMetaData.fileName;
-    this._scriptFileNames = [fileName];
-    const sourceCode = moduleMetaData.sourceCode;
-    let outputCode = moduleMetaData.outputCode;
-    if (!outputCode) {
-      outputCode = moduleMetaData.outputCode = `${this.compile(
-        fileName
-      )}\n//# sourceURL=${fileName}`;
-      moduleMetaData!.scriptVersion = "1";
-      this._os.codeCache(fileName, sourceCode, outputCode);
+    if (!moduleMetaData.hasRun) {
+      const fileName = moduleMetaData.fileName;
+      this._scriptFileNames = [fileName];
+      const sourceCode = moduleMetaData.sourceCode;
+      let outputCode = moduleMetaData.outputCode;
+      if (!outputCode) {
+        outputCode = moduleMetaData.outputCode = `${this.compile(
+          fileName
+        )}\n//# sourceURL=${fileName}`;
+        moduleMetaData.scriptVersion = "1";
+        this._os.codeCache(fileName, sourceCode, outputCode);
+      }
+      this._window.define = this.makeDefine(moduleMetaData);
+      this._globalEval(moduleMetaData.outputCode);
+      this._window.define = undefined;
+      moduleMetaData.hasRun = true;
     }
-    this._window.define = this.makeDefine(moduleMetaData);
-    this._globalEval(moduleMetaData.outputCode);
-    this._window.define = undefined;
-    return moduleMetaData!;
+    return moduleMetaData;
   }

   /**
diff --git a/tests/modules_should_be_executed_once.ts b/tests/modules_should_be_executed_once.ts
new file mode 100644
index 0000000..7b52ad5
--- /dev/null
+++ b/tests/modules_should_be_executed_once.ts
@@ -0,0 +1,2 @@
+import "./subdir/print_hello_on_import.ts";
+import "./subdir/print_hello_on_import.ts";
diff --git a/tests/modules_should_be_executed_once.ts.out b/tests/modules_should_be_executed_once.ts.out
new file mode 100644
index 0000000..e965047
--- /dev/null
+++ b/tests/modules_should_be_executed_once.ts.out
@@ -0,0 +1 @@
+Hello
diff --git a/tests/subdir/print_hello_on_import.ts b/tests/subdir/print_hello_on_import.ts
new file mode 100644
index 0000000..bf6b817
--- /dev/null
+++ b/tests/subdir/print_hello_on_import.ts
@@ -0,0 +1 @@
+console.log("Hello");
-- 
2.15.0

It doesn't quite work tho. Maybe you have a better way of doing this?

[kitsonk]

I am starting on #581 now, which should implicitly resolve this problem, because it will collect all the modules before it evals any of them, and will eval them in a dependency order. There maybe a need to track if they have been eval'ed, but I think it will be a more complete solution than just avoiding this one bug. I should be able to get a working PR today.