From 84e9aad2ace2bad445a3698452c07b2e7f5eb834 Mon Sep 17 00:00:00 2001
From: Doug Parker <dgp1130@users.noreply.github.com>
Date: Tue, 7 Mar 2023 18:41:41 -0800
Subject: [PATCH] fix: always load JUnit reporter as ESM

At runtime, the JUnit reporter exists at `.../execroot/${wksp}/bazel-out/${cfg}/bin/external/jasmine/junit_reporter.js` with no included `package.json`. This means Node walks up that tree looking for a `package.json` file and typically won't find one. However, if users include a `//:package.json` file in their own workspace, it will be visible at `.../execroot/${wksp}/bazel-out/${cfg}/bin/package.json`, meaning it will be picked up as owning `junit_reporter.js`. This means that any user-specified `type: "module"` applies to `junit_reporter.js`, and caused it to be loaded as ESM, even when it was authored in CommonJS.

The solution here is to copy over a `package.json` we own to `external/jasmine/package.json`. This way any JavaScript loaded will have a stable `package.json` which uses CommonJS.

Ironically, `//e2e/jasmine_test/` actually already exercised this exact test case, but wasn't running on CI. So this commit also re-enables that test and fixes it. Actually all the test cases use ESM, so I also added `//examples/commonjs/...` and left the "default" behavior there to ensure that doesn't break in the future.
---
 .github/workflows/ci.yaml        |  1 +
 e2e/jasmine_test/BUILD.bazel     |  1 +
 examples/commonjs/BUILD.bazel    | 11 +++++++++++
 examples/commonjs/lib.js         |  3 +++
 examples/commonjs/lib.spec.js    |  7 +++++++
 jasmine/defs.bzl                 |  1 +
 jasmine/private/BUILD.bazel      |  3 ++-
 jasmine/private/jasmine_test.bzl | 12 ++++++++++++
 jasmine/private/package.json     |  1 +
 jasmine/repositories.bzl         |  6 ++++++
 10 files changed, 45 insertions(+), 1 deletion(-)
 create mode 100644 examples/commonjs/BUILD.bazel
 create mode 100644 examples/commonjs/lib.js
 create mode 100644 examples/commonjs/lib.spec.js
 create mode 100644 jasmine/private/package.json

diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml
index facc12d..a8b9144 100644
--- a/.github/workflows/ci.yaml
+++ b/.github/workflows/ci.yaml
@@ -62,6 +62,7 @@ jobs:
         bazelversion: ${{ fromJSON(needs.matrix-prep-bazelversion.outputs.bazelversions) }}
         folder:
           - "."
+          - "e2e/jasmine_test"
           - "e2e/workspace"
         exclude:
           # Don't test macos with Bazel 5 to minimize macOS minutes (billed at 10X)
diff --git a/e2e/jasmine_test/BUILD.bazel b/e2e/jasmine_test/BUILD.bazel
index 8c6d8b8..e9a6382 100644
--- a/e2e/jasmine_test/BUILD.bazel
+++ b/e2e/jasmine_test/BUILD.bazel
@@ -8,4 +8,5 @@ jasmine_test(
         "lib.test.js",
         "package.json",
     ],
+    args = ["*.spec.js"],
 )
diff --git a/examples/commonjs/BUILD.bazel b/examples/commonjs/BUILD.bazel
new file mode 100644
index 0000000..9c288b0
--- /dev/null
+++ b/examples/commonjs/BUILD.bazel
@@ -0,0 +1,11 @@
+load("@aspect_rules_jasmine//jasmine:defs.bzl", "jasmine_test")
+
+jasmine_test(
+    name = "test",
+    data = [
+        "lib.js",
+        "lib.spec.js",
+    ],
+    args = ["*.spec.js"],
+    chdir = package_name(),
+)
diff --git a/examples/commonjs/lib.js b/examples/commonjs/lib.js
new file mode 100644
index 0000000..801a115
--- /dev/null
+++ b/examples/commonjs/lib.js
@@ -0,0 +1,3 @@
+module.exports = {
+    say_hi: () => "hi",
+};
diff --git a/examples/commonjs/lib.spec.js b/examples/commonjs/lib.spec.js
new file mode 100644
index 0000000..6debdf7
--- /dev/null
+++ b/examples/commonjs/lib.spec.js
@@ -0,0 +1,7 @@
+const { say_hi } = require("./lib.js");
+
+describe("lib", () => {
+    it("should say hi", () => {
+        expect(say_hi()).toBe("hi");
+    });
+});
diff --git a/jasmine/defs.bzl b/jasmine/defs.bzl
index c67bfde..3b5fd65 100644
--- a/jasmine/defs.bzl
+++ b/jasmine/defs.bzl
@@ -19,6 +19,7 @@ def jasmine_test(jasmine_repository = "jasmine", **kwargs):
         }),
         entry_point = "@{}//:jasmine_entrypoint".format(jasmine_repository),
         junit_reporter = "@{}//:junit_reporter".format(jasmine_repository),
+        package_json = "@{}//:package_json".format(jasmine_repository),
         data = kwargs.pop("data", []) + [
             "@{}//:node_modules/jasmine".format(jasmine_repository),
             "@{}//:node_modules/jasmine-core".format(jasmine_repository),
diff --git a/jasmine/private/BUILD.bazel b/jasmine/private/BUILD.bazel
index 55d7a1c..2a2a1b2 100644
--- a/jasmine/private/BUILD.bazel
+++ b/jasmine/private/BUILD.bazel
@@ -1,8 +1,9 @@
 load("@bazel_skylib//:bzl_library.bzl", "bzl_library")
 
 exports_files([
+    "junit_reporter.js",
+    "package.json",
     "runner.js",
-    "junit_reporter.js"
 ])
 
 # gazelle:exclude v*.*.*/*
diff --git a/jasmine/private/jasmine_test.bzl b/jasmine/private/jasmine_test.bzl
index 751a619..9edf34e 100644
--- a/jasmine/private/jasmine_test.bzl
+++ b/jasmine/private/jasmine_test.bzl
@@ -7,6 +7,17 @@ _attrs = dicts.add(js_binary_lib.attrs, {
         mandatory = True,
         allow_single_file = True,
     ),
+    # We include a `package.json` not because it does anything particularly
+    # useful (it doesn't), but because without it, a `package.json` file in the
+    # root of the user's workspace (`//:package.json`) would be used for any
+    # JavaScript invoked in the `@jasmine` workspace. This could break things by
+    # changing the default type to `commonjs` or adding `imports` / `exports`.
+    # Including a `package.json` we own in the test execution, even if empty,
+    # prevents users from accidentally breaking the tests.
+    "package_json": attr.label(
+        mandatory = True,
+        allow_single_file = [".json"],
+    ),
     "config": attr.label(
         doc = "jasmine config file. See: https://jasmine.github.io/setup/nodejs.html#configuration",
         allow_single_file = [".json", ".js"],
@@ -16,6 +27,7 @@ _attrs = dicts.add(js_binary_lib.attrs, {
 def _impl(ctx):
     files = ctx.files.data[:]
     files.append(ctx.file.junit_reporter)
+    files.append(ctx.file.package_json)
 
     junit_reporter = ctx.file.junit_reporter.short_path
 
diff --git a/jasmine/private/package.json b/jasmine/private/package.json
new file mode 100644
index 0000000..0967ef4
--- /dev/null
+++ b/jasmine/private/package.json
@@ -0,0 +1 @@
+{}
diff --git a/jasmine/repositories.bzl b/jasmine/repositories.bzl
index eafc981..d360933 100644
--- a/jasmine/repositories.bzl
+++ b/jasmine/repositories.bzl
@@ -54,6 +54,12 @@ If you need custom versions, please file an issue.""".format(jasmine_version, TO
     src = "@aspect_rules_jasmine//jasmine/private:junit_reporter.js",
     out = "junit_reporter.js",
     visibility = ["//visibility:public"],
+)""",
+                """copy_file(
+    name = "package_json",
+    src = "@aspect_rules_jasmine//jasmine/private:package.json",
+    out = "package.json",
+    visibility = ["//visibility:public"],
 )""",
             ],
             "defs.bzl": [