diff --git a/millbun/integration/resources/scalajs-web/build.mill b/millbun/integration/resources/scalajs-web/build.mill index cbb8d5f..853e71b 100644 --- a/millbun/integration/resources/scalajs-web/build.mill +++ b/millbun/integration/resources/scalajs-web/build.mill @@ -16,6 +16,7 @@ object app extends BunScalaJSWebModule { def scalaVersion = "3.8.2" def scalaJSVersion = "1.22.0" def mvnDeps = Seq(mvn"org.scala-js::scalajs-dom::2.8.1") + override def npmDeps = Task { Seq("lodash@4.17.21") } override def mainClass = Some("Main") override def moduleKind = Task { ModuleKind.ESModule } } diff --git a/millbun/integration/resources/scalajs-web/src/Main.scala b/millbun/integration/resources/scalajs-web/src/Main.scala index 8b563d3..4654008 100644 --- a/millbun/integration/resources/scalajs-web/src/Main.scala +++ b/millbun/integration/resources/scalajs-web/src/Main.scala @@ -1,5 +1,13 @@ import org.scalajs.dom.document +import scala.scalajs.js +import scala.scalajs.js.annotation.JSImport + +/** Forces the linker to emit a real npm import, so the staged web build must resolve it. */ +@js.native +@JSImport("lodash", JSImport.Namespace) +object Lodash extends js.Object: + def capitalize(value: String): String = js.native object Main: def main(args: Array[String]): Unit = - document.getElementById("app").textContent = "Hello from Scala.js web" + document.getElementById("app").textContent = Lodash.capitalize("hello from scala.js web") 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/BunScalaJSIntegrationTests.scala b/millbun/integration/src/mill/bun/BunScalaJSIntegrationTests.scala index ec7bd04..da59704 100644 --- a/millbun/integration/src/mill/bun/BunScalaJSIntegrationTests.scala +++ b/millbun/integration/src/mill/bun/BunScalaJSIntegrationTests.scala @@ -83,6 +83,27 @@ object BunScalaJSIntegrationTests extends TestSuite { assert(files.exists(_.ext == "js")) } + test("web stage resolves npm dependencies") { + // The fixture imports lodash via @JSImport, so the linked output carries a real npm + // import that `bun build` has to resolve out of the staged directory. Staging used to + // flatten the install's node_modules symlink into an empty directory, which made every + // npm import unresolvable. + val tester = this.tester("scalajs-web") + assert(tester.eval("app.bundle").isSuccess) + + val stage = tester.workspacePath / "out" / "app" / "webProductionStage.dest" + assert(os.isLink(stage / "node_modules")) + assert(os.exists(stage / "node_modules" / "lodash" / "package.json")) + + // lodash must be inlined into the bundle, not left as a bare import. + val bundled = os.walk(outputPath(tester, "app.bundle")) + .filter(p => os.isFile(p) && p.ext == "js") + .map(os.read) + .mkString + assert(!bundled.contains("""from"lodash"""")) + assert(!bundled.contains("""from "lodash"""")) + } + test("compileExecutable") { val tester = this.tester("scalajs-bundle") val res = tester.eval("app.compileExecutable") 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 32b0155..af89292 100644 --- a/millbun/src/mill/bun/BunToolchainModule.scala +++ b/millbun/src/mill/bun/BunToolchainModule.scala @@ -299,12 +299,42 @@ object BunToolchainModule { .find(path => os.isFile(path) && java.nio.file.Files.isExecutable(path.toNIO)) } - /** Copy a generated workspace into a fresh task destination, preserving layout. */ - def copyWorkspace(source: os.Path, dest: os.Path): Unit = { - os.walk(source) - .foreach(path => os.copy.over(path, dest / path.relativeTo(source), createFolders = true)) + /** + * Copy a directory tree, recreating symlinks instead of resolving them. + * + * `os.walk` does not follow links, but `os.isDir` and `os.copy` do. Branching on one while + * copying with the other means a `node_modules` symlink is either deep-copied (turning an O(1) + * link into an O(node_modules) copy) or flattened into an empty directory, and a broken link + * anywhere in the tree — routine for the `.bin` shims of skipped optional dependencies — aborts + * the whole copy. Recreating the link is both correct and cheap. + * + * @param exclude top-level entry names to skip entirely + */ + def copyTree(source: os.Path, dest: os.Path, exclude: Set[String] = Set.empty): Unit = { + if (!os.exists(source, followLinks = false)) return + + val skip = (path: os.Path) => { + val relative = path.relativeTo(source) + relative.segments.headOption.exists(exclude.contains) + } + + os.walk(source, skip = skip, followLinks = false).foreach { path => + val target = dest / path.relativeTo(source) + if (os.isLink(path)) { + os.makeDir.all(target / os.up) + if (os.exists(target, followLinks = false)) os.remove(target) + os.symlink(target, os.readLink.absolute(path)) + } else if (os.isDir(path, followLinks = false)) { + os.makeDir.all(target) + } else { + os.copy.over(path, target, createFolders = true) + } + } } + /** Copy a generated workspace into a fresh task destination, preserving layout and symlinks. */ + def copyWorkspace(source: os.Path, dest: os.Path): Unit = copyTree(source, dest) + /** * Copy files or directories into a Bun workspace while preserving their relative path * beneath the nearest matching source root when possible. @@ -322,13 +352,8 @@ object BunToolchainModule { .map(root => destRoot / source.relativeTo(root)) .getOrElse(destRoot / source.last) - if (os.isDir(source)) { - os.walk(source).foreach { path => - os.copy.over(path, target / path.relativeTo(source), createFolders = true) - } - } else { - os.copy.over(source, target, createFolders = true) - } + if (os.isDir(source)) copyTree(source, target) + else os.copy.over(source, target, createFolders = true) } } } @@ -455,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/bun/BunWebSupport.scala b/millbun/src/mill/bun/BunWebSupport.scala index a64af40..0b779c1 100644 --- a/millbun/src/mill/bun/BunWebSupport.scala +++ b/millbun/src/mill/bun/BunWebSupport.scala @@ -13,32 +13,29 @@ private[mill] object BunWebSupport: copyPath(source, target) } - def copyContents(source: os.Path, destination: os.Path): Unit = + def copyContents( + source: os.Path, + destination: os.Path, + exclude: Set[String] = Set.empty + ): Unit = if os.exists(source) then - if os.isDir(source) then - os.walk(source).foreach { path => - val relative = path.relativeTo(source) - if relative.segments.nonEmpty then - val target = destination / relative - if os.isDir(path) then os.makeDir.all(target) - else os.copy.over(path, target, createFolders = true) - } + if os.isDir(source) then BunToolchainModule.copyTree(source, destination, exclude) else copyPath(source, destination / source.last) private def copyPath(source: os.Path, target: os.Path): Unit = - if os.isDir(source) then - os.walk(source).foreach { path => - val destination = target / path.relativeTo(source) - if os.isDir(path) then os.makeDir.all(destination) - else os.copy.over(path, destination, createFolders = true) - } + if os.isDir(source) then BunToolchainModule.copyTree(source, target) else os.copy.over(source, target, createFolders = true) + /** + * Resolve the HTML entrypoints a staged web build will use. Pure — writes nothing. + * + * Kept separate from [[materializeHtmlEntries]] so `dev()` and `bundle` can ask which entries + * exist without writing into the staging task's already-cached output directory. + */ def htmlEntries( configured: Seq[PathRef], moduleDir: os.Path, - destination: os.Path, - generatedScript: String + destination: os.Path ): Seq[os.Path] = val copied = configured .map(_.path) @@ -48,11 +45,20 @@ private[mill] object BunWebSupport: else destination / path.last ) - if copied.nonEmpty then copied - else - val index = destination / "index.html" + if copied.nonEmpty then copied else Seq(destination / "index.html") + + /** Resolve entrypoints, generating a minimal `index.html` when the module supplies none. */ + def materializeHtmlEntries( + configured: Seq[PathRef], + moduleDir: os.Path, + destination: os.Path, + generatedScript: String + ): Seq[os.Path] = + val entries = htmlEntries(configured, moduleDir, destination) + val hasConfigured = configured.exists(ref => os.exists(ref.path) && os.isFile(ref.path)) + if !hasConfigured then os.write.over( - index, + entries.head, s""" | |
@@ -61,7 +67,7 @@ private[mill] object BunWebSupport: |""".stripMargin, createFolders = true ) - Seq(index) + entries def syncRoots(roots: Seq[(os.Path, os.Path)]): Unit = roots.foreach { case (source, target) => diff --git a/millbun/src/mill/bun/BunWorkersModule.scala b/millbun/src/mill/bun/BunWorkersModule.scala index 8e1934d..cb50d8a 100644 --- a/millbun/src/mill/bun/BunWorkersModule.scala +++ b/millbun/src/mill/bun/BunWorkersModule.scala @@ -59,6 +59,9 @@ trait BunWorkersModule extends BunToolchainModule { this: BunTypeScriptModule => def bundleWorkers: T[PathRef] = Task { val workspace = Task.dest / "workspace" val outDir = Task.dest / "workers" + // Declared explicitly: the staged tree carries a node_modules symlink into this install, + // and Mill's filesystem checker only permits reading a dest we depend on. + npmInstall() BunToolchainModule.copyWorkspace(compile().path, workspace) os.makeDir.all(outDir) diff --git a/millbun/src/mill/javascriptlib/bun/BunTypeScriptModule.scala b/millbun/src/mill/javascriptlib/bun/BunTypeScriptModule.scala index 170e922..44644f0 100644 --- a/millbun/src/mill/javascriptlib/bun/BunTypeScriptModule.scala +++ b/millbun/src/mill/javascriptlib/bun/BunTypeScriptModule.scala @@ -284,6 +284,9 @@ trait BunTypeScriptModule extends TypeScriptModule with BunToolchainModule with if (bunCompileExecutable()) Task.dest / bunBinaryName() else Task.dest / s"$moduleName.js" + // Declared explicitly: the staged tree carries a node_modules symlink into this + // install, and Mill's filesystem checker only permits reading a dest we depend on. + npmInstall() BunToolchainModule.copyWorkspace(compileDir, buildDir) BunTypeScriptModule.removeInstallOnlyConfigs(buildDir) BunTypeScriptModule.copyBunfigsTo(buildDir, resolvedBunfigs()) @@ -319,6 +322,9 @@ trait BunTypeScriptModule extends TypeScriptModule with BunToolchainModule with val mainFile = resolvedEntrypoint(mainFilePath(), compileDir).relativeTo(compileDir).toString val outFile = Task.dest / bunBinaryName() + // Declared explicitly: the staged tree carries a node_modules symlink into this + // install, and Mill's filesystem checker only permits reading a dest we depend on. + npmInstall() BunToolchainModule.copyWorkspace(compileDir, buildDir) BunTypeScriptModule.removeInstallOnlyConfigs(buildDir) BunTypeScriptModule.copyBunfigsTo(buildDir, resolvedBunfigs()) @@ -356,6 +362,9 @@ trait BunTypeScriptModule extends TypeScriptModule with BunToolchainModule with val compileDir = compile().path val buildDir = Task.dest / "workspace" val mainFile = resolvedEntrypoint(mainFilePath(), compileDir).relativeTo(compileDir).toString + // Declared explicitly: the staged tree carries a node_modules symlink into this + // install, and Mill's filesystem checker only permits reading a dest we depend on. + npmInstall() BunToolchainModule.copyWorkspace(compileDir, buildDir) BunTypeScriptModule.removeInstallOnlyConfigs(buildDir) BunTypeScriptModule.copyBunfigsTo(buildDir, resolvedBunfigs()) @@ -409,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( @@ -447,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 `