diff --git a/millbun/integration/resources/typescript-test-deps/build.mill b/millbun/integration/resources/typescript-test-deps/build.mill index cd619a0..a315dd8 100644 --- a/millbun/integration/resources/typescript-test-deps/build.mill +++ b/millbun/integration/resources/typescript-test-deps/build.mill @@ -11,11 +11,16 @@ import mill.javascriptlib.bun.* object app extends BunTypeScriptModule { override def moduleDir = build.moduleDir + // Pinned in the fixture rather than relying on the suite's env default, so the strict + // lockfile path is exercised regardless of how the suite is invoked. + override def bunRequireLockfile = Task { true } + // Production dependency override def npmDeps = Task { Seq("is-even@1.0.0") } object test extends BunTypeScriptTests { - // Test-only dependency — should land in devDependencies, not dependencies + // Test-only dependency — should land in devDependencies, not dependencies. Because it is a + // dependency the outer module does not have, this test module needs its own bun.lock. override def npmDeps = Task { Seq("is-odd@3.0.1") } } } diff --git a/millbun/integration/src/mill/bun/BunTypeScriptIntegrationTests.scala b/millbun/integration/src/mill/bun/BunTypeScriptIntegrationTests.scala index ae52d1a..0aa008e 100644 --- a/millbun/integration/src/mill/bun/BunTypeScriptIntegrationTests.scala +++ b/millbun/integration/src/mill/bun/BunTypeScriptIntegrationTests.scala @@ -178,6 +178,9 @@ object BunTypeScriptIntegrationTests extends TestSuite { test("test deps are devDependencies") { val tester = this.tester("typescript-test-deps") + // The fixture sets bunRequireLockfile, so both modules need a lock before installing. + assert(tester.eval("app.bunLock").isSuccess) + assert(tester.eval("app.test.bunLock").isSuccess) // Outer module should have is-even in dependencies val outerRes = tester.eval("app.npmInstall") @@ -201,6 +204,47 @@ object BunTypeScriptIntegrationTests extends TestSuite { assert(runRes.isSuccess) } + test("test modules with extra deps own their lockfile") { + // A test module installs a strict superset of the outer package.json. Reusing the outer + // module's lock under --frozen-lockfile fails with "lockfile had changes, but lockfile is + // frozen", so the test module needs its own lock and its own bunLock command. + val tester = this.tester("typescript-test-deps") + + // Without any lock, the install refuses and names the test module's own path. + val unlocked = tester.eval("app.test.npmInstall") + assert(!unlocked.isSuccess) + + assert(tester.eval("app.bunLock").isSuccess) + assert(tester.eval("app.test.bunLock").isSuccess) + + val outerLock = tester.workspacePath / "bun.lock" + val testLock = tester.workspacePath / "test" / "bun.lock" + assert(os.exists(outerLock)) + assert(os.exists(testLock)) + assert(os.read(outerLock) != os.read(testLock)) + + // Compare the `workspaces` block, which records the root package's *declared* deps. + // is-odd also arrives transitively through is-even, so a whole-file substring match + // would not distinguish the two locks. + def declaredDeps(lock: os.Path): String = + val text = os.read(lock) + text.slice(text.indexOf("\"workspaces\""), text.indexOf("\"packages\"")) + + assert(!declaredDeps(outerLock).contains("is-odd")) + assert(declaredDeps(testLock).contains("is-odd")) + + // The frozen install now succeeds against the test module's own lock. + assert(tester.eval("app.test.npmInstall").isSuccess) + assert(tester.eval("app.test.test").isSuccess) + } + + test("test modules adding nothing reuse the outer install") { + // A bare test module must not demand a second lockfile. + val tester = this.tester("typescript-tests") + assert(tester.eval("app.test.npmInstall").isSuccess) + assert(!os.exists(tester.workspacePath / "test" / "bun.lock")) + } + test("bunEnv") { val tester = this.tester("typescript-env") val res = tester.eval("app.bundle") diff --git a/millbun/src/mill/bun/BunToolchainModule.scala b/millbun/src/mill/bun/BunToolchainModule.scala index 966abeb..af89292 100644 --- a/millbun/src/mill/bun/BunToolchainModule.scala +++ b/millbun/src/mill/bun/BunToolchainModule.scala @@ -480,14 +480,19 @@ trait BunToolchainModule extends Module { else Seq.empty) } + /** + * @param lockfilePath where the missing lockfile is expected. Nested modules must pass their own + * path, or the error points a user at the wrong file. + */ protected def requireBunLockfile( hasInstallInputs: Boolean, lockfile: Option[PathRef], - required: Boolean + required: Boolean, + lockfilePath: os.Path = moduleDir / "bun.lock" ): Unit = { if (hasInstallInputs && required && lockfile.isEmpty) { throw new RuntimeException( - s"Missing ${moduleDir / "bun.lock"}. Run this module's bunLock command and commit the generated lockfile." + s"Missing $lockfilePath. Run this module's bunLock command and commit the generated lockfile." ) } } diff --git a/millbun/src/mill/javascriptlib/bun/BunTypeScriptModule.scala b/millbun/src/mill/javascriptlib/bun/BunTypeScriptModule.scala index 1141d92..44644f0 100644 --- a/millbun/src/mill/javascriptlib/bun/BunTypeScriptModule.scala +++ b/millbun/src/mill/javascriptlib/bun/BunTypeScriptModule.scala @@ -418,23 +418,26 @@ trait BunTypeScriptModule extends TypeScriptModule with BunToolchainModule with /** Coverage reporter formats. */ def bunCoverageReporters: T[Seq[String]] = Task { Seq("text", "lcov") } - override def npmInstall: T[PathRef] = Task { - val dest = Task.dest - os.makeDir.all(dest) - - // Merge outer + test-side deps into a single package.json. - // Upstream Mill's test npmInstall runs `npm install --save-dev` with the - // test module's transitive deps; we achieve the same by building one - // merged package.json before `bun install`. + /** + * Merged outer + test-side package.json, shared by [[npmInstall]] and [[bunLock]]. + * + * One task so the install and the lockfile can never describe different dependency sets — + * that divergence is what made frozen installs of test modules fail. + * + * Upstream Mill's test `npmInstall` runs `npm install --save-dev` with the test module's + * transitive deps; building one merged package.json achieves the same for Bun. + */ + def bunTestPackageJson: T[ujson.Obj] = Task { val user = outer.packageJson() val overrides = outer.npmOverrides() val outerDeps = BunToolchainModule.dependencyPairs(outer.transitiveNpmDeps(), overrides) - val outerDevDeps = BunToolchainModule.dependencyPairs(outer.transitiveNpmDevDeps() ++ outer.tsDeps(), overrides) + val outerDevDeps = + BunToolchainModule.dependencyPairs(outer.transitiveNpmDevDeps() ++ outer.tsDeps(), overrides) val outerPackageNames = (outerDeps.iterator ++ outerDevDeps.iterator).map(_._1).toSet // Test-only deps are dev dependencies — they should not appear in the // production dependencies field, matching Bun/npm convention. val testDevDeps = BunToolchainModule - .dependencyPairs(transitiveNpmDeps() ++ this.npmDevDeps() ++ this.tsDeps(), overrides) + .dependencyPairs(this.transitiveNpmDeps() ++ this.npmDevDeps() ++ this.tsDeps(), overrides) .filterNot { case (name, _) => outerPackageNames.contains(name) } val resolved = ujson.Obj.from( @@ -456,28 +459,99 @@ trait BunTypeScriptModule extends TypeScriptModule with BunToolchainModule with overrides.toSeq.sortBy(_._1).map((name, value) => name -> ujson.Str(value)) ) - val merged = BunToolchainModule.mergePackageJson(resolved, outer.bunPackageJsonExtras()) - os.write.over(dest / "package.json", merged.render(indent = 2), createFolders = true) + BunToolchainModule.mergePackageJson(resolved, outer.bunPackageJsonExtras()) + } - outer.copyBunWorkspaceConfigs() + /** + * Source-controlled lockfile for this test module, at `/bun.lock`. + * + * Declared here rather than inherited from the enclosing module: a test module that adds + * dependencies installs a strict superset of the outer package.json, which the outer module's + * lockfile cannot satisfy under `--frozen-lockfile`. + */ + def bunLockfile: T[Option[PathRef]] = Task.Input { + val path = moduleDir / "bun.lock" + if (os.exists(path)) Some(PathRef(path)) else None + } - val lockfile = bunLockfile() - requireBunLockfile(true, lockfile, bunRequireLockfile()) - copyBunLockfile(lockfile, dest) + /** True when the test module adds nothing the outer install does not already provide. */ + private def reusesOuterInstall: Task[Boolean] = Task.Anon { + bunTestPackageJson().render() == outer.bunWorkspacePackageJson().render() + } - runBun( - bunExecutable(), - Seq("install") ++ resolvedBunInstallArgs( - bunInstallArgs(), - bunInstallExtraArgs(), - lockfile.nonEmpty, - updateLockfile = false - ) ++ (outer.transitiveUnmanagedDeps() ++ transitiveUnmanagedDeps()).distinct.map(_.path.toString), + override def npmInstall: T[PathRef] = Task { + if (reusesOuterInstall()) outer.npmInstall() + else { + val dest = Task.dest + os.makeDir.all(dest) + os.write.over( + dest / "package.json", + bunTestPackageJson().render(indent = 2), + createFolders = true + ) + outer.copyBunWorkspaceConfigs() + + val lockfile = this.bunLockfile() + outer.requireBunLockfile( + hasInstallInputs = true, + lockfile = lockfile, + required = outer.bunRequireLockfile(), + lockfilePath = moduleDir / "bun.lock" + ) + outer.copyBunLockfile(lockfile, dest) + + outer.runBun( + outer.bunExecutable(), + Seq("install") ++ outer.resolvedBunInstallArgs( + outer.bunInstallArgs(), + outer.bunInstallExtraArgs(), + lockfile.nonEmpty, + updateLockfile = false + ) ++ (outer.transitiveUnmanagedDeps() ++ this.transitiveUnmanagedDeps()) + .distinct.map(_.path.toString), + cwd = dest, + env = outer.bunToolEnv() + ) + + PathRef(dest) + } + } + + /** + * Resolve this test module's dependencies and update its own `bun.lock`. + * + * Unlike the outer module's, this does not refuse for workspace members: a test module with + * extra dependencies genuinely needs its own install and its own lock. + */ + def bunLock(): Command[PathRef] = Task.Command { + val dest = Task.dest + os.makeDir.all(dest) + os.write.over( + dest / "package.json", + bunTestPackageJson().render(indent = 2), + createFolders = true + ) + outer.copyBunWorkspaceConfigs() + outer.copyBunLockfile(this.bunLockfile(), dest) + + outer.runBun( + outer.bunExecutable(), + Seq("install") ++ outer.resolvedBunInstallArgs( + outer.bunInstallArgs(), + outer.bunInstallExtraArgs(), + this.bunLockfile().nonEmpty, + updateLockfile = true + ) ++ (outer.transitiveUnmanagedDeps() ++ this.transitiveUnmanagedDeps()) + .distinct.map(_.path.toString), cwd = dest, env = outer.bunToolEnv() ) - PathRef(dest) + val generated = dest / "bun.lock" + if (!os.exists(generated)) Task.fail("Bun did not generate bun.lock") + val sourceLock = moduleDir / "bun.lock" + os.copy.over(generated, sourceLock, createFolders = true) + PathRef(sourceLock) } protected def preparedTestWorkspace: T[PathRef] = Task {