diff --git a/CHANGELOG.md b/CHANGELOG.md index 9269351f0b..62eff0643e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,6 +17,7 @@ - Remove the legacy build system. Going forward, only the modern build system is supported, and the `rescript-legacy` command is not available anymore. https://github.com/rescript-lang/rescript/pull/8186 - `Int.fromString` and `Float.fromString` use stricter number parsing and no longer uses an explicit radix argument, but instead supports parsing hexadecimal, binary and exponential notation. - Remove `external-stdlib` configuration option from `rescript.json`. This option was rarely used and is no longer supported. +- `js-post-build` now passes the correct output file path based on `in-source` configuration: when `in-source: true`, the path next to the source file is passed; when `in-source: false`, the path in the `lib//` directory is passed. Additionally, stdout and stderr from the post-build command are now logged. https://github.com/rescript-lang/rescript/pull/8190 #### :eyeglasses: Spec Compliance diff --git a/docs/docson/build-schema.json b/docs/docson/build-schema.json index c79b88b530..0e3155343a 100644 --- a/docs/docson/build-schema.json +++ b/docs/docson/build-schema.json @@ -124,7 +124,8 @@ "type": "object", "properties": { "cmd": { - "type": "string" + "type": "string", + "description": "Shell command to run after each JS file is compiled. The absolute path to the output JS file is appended as an argument. The path respects the `in-source` setting." } } }, @@ -453,7 +454,7 @@ }, "js-post-build": { "$ref": "#/definitions/js-post-build", - "description": "(Experimental) post-processing hook. The build system will invoke `cmd ${file}` whenever a `${file}` is changed" + "description": "Post-processing hook. The build system will invoke `cmd ` after each JS file is compiled. The path respects the `in-source` setting: when true, the path is next to the source file; when false, the path is in the `lib//` directory. The command runs with the same working directory as the build process (typically the project root)." }, "package-specs": { "$ref": "#/definitions/package-specs", diff --git a/rewatch/CompilerConfigurationSpec.md b/rewatch/CompilerConfigurationSpec.md index f3ec4b0889..d5b7287336 100644 --- a/rewatch/CompilerConfigurationSpec.md +++ b/rewatch/CompilerConfigurationSpec.md @@ -2,35 +2,35 @@ This document contains a list of all bsconfig parameters with remarks, and whether they are already implemented in rewatch. It is based on https://rescript-lang.org/docs/manual/latest/build-configuration-schema. -| Parameter | JSON type | Remark | Implemented? | -| --------------------- | ----------------------- | ---------------------------------------- | :----------: | -| name | string | | [x] | -| namespace | boolean | | [x] | -| namespace | string | | [x] | -| sources | string | | [x] | -| sources | array of string | | [x] | -| sources | Source | | [x] | -| sources | array of Source | | [x] | -| ignored-dirs | array of string | | [_] | -| dependencies | array of string | | [x] | -| dev-dependencies | array of string | | [x] | -| generators | array of Rule-Generator | | [_] | -| cut-generators | boolean | | [_] | -| jsx | JSX | | [x] | -| gentypeconfig | Gentype | | [x] | -| compiler-flags | array of string | | [x] | -| warnings | Warnings | | [x] | -| ppx-flags | array of string | | [x] | -| pp-flags | array of string | | [_] | -| js-post-build | Js-Post-Build | `${file}` is now an absolute path | [x] | -| package-specs | array of Module-Format | | [_] | -| package-specs | array of Package-Spec | | [x] | -| entries | array of Target-Item | | [_] | -| bs-external-includes | array of string | | [_] | -| suffix | Suffix | | [x] | -| reanalyze | Reanalyze | Reanalyze config; ignored by rewatch | [x] | -| experimental-features | ExperimentalFeatures | | [x] | -| editor | object | VS Code tooling only; ignored by rewatch | [x] | +| Parameter | JSON type | Remark | Implemented? | +| --------------------- | ----------------------- | ----------------------------------------------------------- | :----------: | +| name | string | | [x] | +| namespace | boolean | | [x] | +| namespace | string | | [x] | +| sources | string | | [x] | +| sources | array of string | | [x] | +| sources | Source | | [x] | +| sources | array of Source | | [x] | +| ignored-dirs | array of string | | [_] | +| dependencies | array of string | | [x] | +| dev-dependencies | array of string | | [x] | +| generators | array of Rule-Generator | | [_] | +| cut-generators | boolean | | [_] | +| jsx | JSX | | [x] | +| gentypeconfig | Gentype | | [x] | +| compiler-flags | array of string | | [x] | +| warnings | Warnings | | [x] | +| ppx-flags | array of string | | [x] | +| pp-flags | array of string | | [_] | +| js-post-build | Js-Post-Build | Path respects `in-source` setting; stdout/stderr are logged | [x] | +| package-specs | array of Module-Format | | [_] | +| package-specs | array of Package-Spec | | [x] | +| entries | array of Target-Item | | [_] | +| bs-external-includes | array of string | | [_] | +| suffix | Suffix | | [x] | +| reanalyze | Reanalyze | Reanalyze config; ignored by rewatch | [x] | +| experimental-features | ExperimentalFeatures | | [x] | +| editor | object | VS Code tooling only; ignored by rewatch | [x] | ### Source @@ -133,7 +133,16 @@ Currently supported features: | Parameter | JSON type | Remark | Implemented? | | --------- | --------- | --------------------------------- | :----------: | -| cmd | string | `${file}` is now an absolute path | [x] | +| cmd | string | Receives absolute path to JS file | [x] | + +The path passed to the command respects the `in-source` setting: + +- `in-source: true` → path next to the source file (e.g., `src/Foo.js`) +- `in-source: false` → path in `lib//` directory (e.g., `lib/es6/src/Foo.mjs`) + +The command runs with the same working directory as the rewatch process (typically the project root). + +stdout and stderr from the command are logged. ### Package-Spec diff --git a/rewatch/src/build/compile.rs b/rewatch/src/build/compile.rs index f9e7baf36d..25f9b661fc 100644 --- a/rewatch/src/build/compile.rs +++ b/rewatch/src/build/compile.rs @@ -13,7 +13,7 @@ use crate::project_context::ProjectContext; use ahash::{AHashMap, AHashSet}; use anyhow::{Result, anyhow}; use console::style; -use log::{debug, trace}; +use log::{debug, info, trace, warn}; use rayon::prelude::*; use std::path::Path; use std::path::PathBuf; @@ -26,6 +26,8 @@ use std::time::SystemTime; fn execute_post_build_command(cmd: &str, js_file_path: &Path) -> Result<()> { let full_command = format!("{} {}", cmd, js_file_path.display()); + debug!("Executing js-post-build: {}", full_command); + let output = if cfg!(target_os = "windows") { Command::new("cmd").args(["/C", &full_command]).output() } else { @@ -33,18 +35,29 @@ fn execute_post_build_command(cmd: &str, js_file_path: &Path) -> Result<()> { }; match output { - Ok(output) if !output.status.success() => { + Ok(output) => { let stderr = String::from_utf8_lossy(&output.stderr); let stdout = String::from_utf8_lossy(&output.stdout); - Err(anyhow!( - "js-post-build command failed for {}: {}{}", - js_file_path.display(), - stderr, - stdout - )) + + // Always log stdout/stderr - the user explicitly configured this command + // and likely cares about its output + if !stdout.is_empty() { + info!("{}", stdout.trim()); + } + if !stderr.is_empty() { + warn!("{}", stderr.trim()); + } + + if !output.status.success() { + Err(anyhow!( + "js-post-build command failed for {}", + js_file_path.display() + )) + } else { + Ok(()) + } } Err(e) => Err(anyhow!("Failed to execute js-post-build command: {}", e)), - Ok(_) => Ok(()), } } @@ -853,10 +866,23 @@ fn compile_file( { // Execute post-build command for each package spec (each output format) for spec in root_config.get_package_specs() { - let js_file = helpers::get_source_file_from_rescript_file( - &package.get_build_path().join(path), - &root_config.get_suffix(&spec), - ); + // Determine the correct JS file path based on in-source setting: + // - in-source: true -> next to the source file (e.g., src/Foo.js) + // - in-source: false -> in lib// directory (e.g., lib/es6/src/Foo.js) + let js_file = if spec.in_source { + helpers::get_source_file_from_rescript_file( + &Path::new(&package.path).join(path), + &root_config.get_suffix(&spec), + ) + } else { + helpers::get_source_file_from_rescript_file( + &Path::new(&package.path) + .join("lib") + .join(spec.get_out_of_source_dir()) + .join(path), + &root_config.get_suffix(&spec), + ) + }; if js_file.exists() { // Fail the build if post-build command fails (matches bsb behavior with &&) diff --git a/tests/build_tests/post-build-out-of-source/input.js b/tests/build_tests/post-build-out-of-source/input.js new file mode 100644 index 0000000000..c5d3a8ee02 --- /dev/null +++ b/tests/build_tests/post-build-out-of-source/input.js @@ -0,0 +1,51 @@ +// @ts-check + +import * as assert from "node:assert"; +import * as fs from "node:fs"; +import * as path from "node:path"; +import { setup } from "#dev/process"; + +const { execBuild, execClean } = setup(import.meta.dirname); + +const isWindows = process.platform === "win32"; + +const logFile = path.join(import.meta.dirname, "post-build-paths.txt"); + +// Clean up any previous log file +if (fs.existsSync(logFile)) { + fs.unlinkSync(logFile); +} + +const out = await execBuild(); + +if (out.status !== 0) { + assert.fail(out.stdout + out.stderr); +} + +try { + // Verify that the post-build command received the correct paths + assert.ok(fs.existsSync(logFile), "post-build-paths.txt should exist"); + + const paths = fs.readFileSync(logFile, "utf-8").trim().split("\n"); + + // With in-source: false and module: esmodule, the paths should be in lib/es6/ + // e.g., /path/to/post-build-out-of-source/lib/es6/src/demo.mjs (Unix) + // e.g., C:\path\to\post-build-out-of-source\lib\es6\src\demo.mjs (Windows) + const libEs6Sep = isWindows ? "\\lib\\es6\\" : "/lib/es6/"; + const libBsSep = isWindows ? "\\lib\\bs\\" : "/lib/bs/"; + + for (const p of paths) { + assert.ok( + p.includes(libEs6Sep) && p.endsWith(".mjs"), + `Path should be in lib/es6/ directory with .mjs suffix: ${p}`, + ); + // Should NOT be in lib/bs/ when in-source is false + assert.ok(!p.includes(libBsSep), `Path should not be in lib/bs/: ${p}`); + } +} finally { + // Clean up log file + if (fs.existsSync(logFile)) { + fs.unlinkSync(logFile); + } + await execClean(); +} diff --git a/tests/build_tests/post-build-out-of-source/log-path.js b/tests/build_tests/post-build-out-of-source/log-path.js new file mode 100644 index 0000000000..76d0e7b73d --- /dev/null +++ b/tests/build_tests/post-build-out-of-source/log-path.js @@ -0,0 +1,6 @@ +import * as fs from "node:fs"; +import * as path from "node:path"; + +const jsFile = process.argv[2]; +const logFile = path.join(import.meta.dirname, "post-build-paths.txt"); +fs.appendFileSync(logFile, jsFile + "\n"); diff --git a/tests/build_tests/post-build-out-of-source/rescript.json b/tests/build_tests/post-build-out-of-source/rescript.json new file mode 100644 index 0000000000..f2be33dea9 --- /dev/null +++ b/tests/build_tests/post-build-out-of-source/rescript.json @@ -0,0 +1,16 @@ +{ + "name": "post-build-out-of-source", + "sources": { + "dir": "src", + "subdirs": true + }, + "package-specs": { + "module": "esmodule", + "in-source": false + }, + "suffix": ".mjs", + "js-post-build": { "cmd": "node ./log-path.js" }, + "warnings": { + "error": "+101" + } +} diff --git a/tests/build_tests/post-build-out-of-source/src/demo.res b/tests/build_tests/post-build-out-of-source/src/demo.res new file mode 100644 index 0000000000..0547b3d0ee --- /dev/null +++ b/tests/build_tests/post-build-out-of-source/src/demo.res @@ -0,0 +1 @@ +let x = 1 diff --git a/tests/build_tests/post-build/input.js b/tests/build_tests/post-build/input.js index 97c5c5c5ea..9c8d843c68 100644 --- a/tests/build_tests/post-build/input.js +++ b/tests/build_tests/post-build/input.js @@ -1,13 +1,20 @@ // @ts-check import * as assert from "node:assert"; +import * as fs from "node:fs"; +import * as path from "node:path"; import { setup } from "#dev/process"; const { execBuild, execClean } = setup(import.meta.dirname); -if (process.platform === "win32") { - console.log("Skipping test on Windows"); - process.exit(0); +const isWindows = process.platform === "win32"; + +// Use a node script for cross-platform path logging +const logFile = path.join(import.meta.dirname, "post-build-paths.txt"); + +// Clean up any previous log file +if (fs.existsSync(logFile)) { + fs.unlinkSync(logFile); } const out = await execBuild(); @@ -16,4 +23,33 @@ if (out.status !== 0) { assert.fail(out.stdout + out.stderr); } -await execClean(); +try { + // Verify that the post-build command received the correct paths + assert.ok(fs.existsSync(logFile), "post-build-paths.txt should exist"); + + const paths = fs.readFileSync(logFile, "utf-8").trim().split("\n"); + + // With in-source: true, the paths should be next to the source files + // e.g., /path/to/post-build/src/demo.js (Unix) + // e.g., C:\path\to\post-build\src\demo.js (Windows) + const srcSep = isWindows ? "\\src\\" : "/src/"; + const libBsSep = isWindows ? "\\lib\\bs\\" : "/lib/bs/"; + + for (const p of paths) { + assert.ok( + p.includes(srcSep) && p.endsWith(".js"), + `Path should be in src/ directory: ${p}`, + ); + // Should NOT be in lib/bs/ when in-source is true + assert.ok( + !p.includes(libBsSep), + `Path should not be in lib/bs/ when in-source is true: ${p}`, + ); + } +} finally { + // Clean up log file + if (fs.existsSync(logFile)) { + fs.unlinkSync(logFile); + } + await execClean(); +} diff --git a/tests/build_tests/post-build/log-path.js b/tests/build_tests/post-build/log-path.js new file mode 100644 index 0000000000..76d0e7b73d --- /dev/null +++ b/tests/build_tests/post-build/log-path.js @@ -0,0 +1,6 @@ +import * as fs from "node:fs"; +import * as path from "node:path"; + +const jsFile = process.argv[2]; +const logFile = path.join(import.meta.dirname, "post-build-paths.txt"); +fs.appendFileSync(logFile, jsFile + "\n"); diff --git a/tests/build_tests/post-build/rescript.json b/tests/build_tests/post-build/rescript.json index 8d9dc159d2..b3e87bf203 100644 --- a/tests/build_tests/post-build/rescript.json +++ b/tests/build_tests/post-build/rescript.json @@ -4,7 +4,11 @@ "dir": "src", "subdirs": true }, - "js-post-build": { "cmd": "cat" }, + "package-specs": { + "module": "commonjs", + "in-source": true + }, + "js-post-build": { "cmd": "node ./log-path.js" }, "warnings": { "error": "+101" } diff --git a/tests/build_tests/post-build/src/demo.res b/tests/build_tests/post-build/src/demo.res index 8d0b19151f..3bfb9a1da2 100644 --- a/tests/build_tests/post-build/src/demo.res +++ b/tests/build_tests/post-build/src/demo.res @@ -1 +1 @@ -let () = Js.log("Hello, ReScript") +let () = Console.log("Hello, ReScript")