Skip to content

Commit 0bfe571

Browse files
nojafcknitt
authored andcommitted
Improve js-post-build (#8190)
* Use correct js path * Print stdout/stderr of js-post-build * Add changelog * Update documentation * Mention the working dir as well # Conflicts: # CHANGELOG.md # docs/docson/build-schema.json # rewatch/CompilerConfigurationSpec.md # tests/build_tests/post-build/input.js
1 parent 5010fbb commit 0bfe571

File tree

12 files changed

+185
-25
lines changed

12 files changed

+185
-25
lines changed

CHANGELOG.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,8 @@
1414

1515
#### :boom: Breaking Change
1616

17+
- `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/<module>/` directory is passed. Additionally, stdout and stderr from the post-build command are now logged. https://github.com/rescript-lang/rescript/pull/8190
18+
1719
#### :eyeglasses: Spec Compliance
1820

1921
#### :rocket: New Feature

docs/docson/build-schema.json

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -124,7 +124,8 @@
124124
"type": "object",
125125
"properties": {
126126
"cmd": {
127-
"type": "string"
127+
"type": "string",
128+
"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."
128129
}
129130
}
130131
},
@@ -454,7 +455,7 @@
454455
},
455456
"js-post-build": {
456457
"$ref": "#/definitions/js-post-build",
457-
"description": "(Experimental) post-processing hook. bsb will invoke `cmd ${file}` whenever a `${file}` is changed"
458+
"description": "Post-processing hook. The build system will invoke `cmd <absolute-path-to-js-file>` 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/<module>/` directory. The command runs with the same working directory as the build process (typically the project root)."
458459
},
459460
"package-specs": {
460461
"$ref": "#/definitions/package-specs",

rewatch/CompilerConfigurationSpec.md

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ This document contains a list of all bsconfig parameters with remarks, and wheth
2222
| warnings | Warnings | | [x] |
2323
| ppx-flags | array of string | | [x] |
2424
| pp-flags | array of string | | [_] |
25-
| js-post-build | Js-Post-Build | `${file}` is now an absolute path | [x] |
25+
| js-post-build | Js-Post-Build | Path respects `in-source` setting; stdout/stderr are logged | [x] |
2626
| package-specs | array of Module-Format | | [_] |
2727
| package-specs | array of Package-Spec | | [x] |
2828
| entries | array of Target-Item | | [_] |
@@ -133,9 +133,18 @@ Currently supported features:
133133

134134
### Js-Post-Build
135135

136-
| Parameter | JSON type | Remark | Implemented? |
137-
| --------- | --------- | ------ | :----------: |
138-
| cmd | string | `${file}` is now an absolute path | [x] |
136+
| Parameter | JSON type | Remark | Implemented? |
137+
| --------- | --------- | --------------------------------- | :----------: |
138+
| cmd | string | Receives absolute path to JS file | [x] |
139+
140+
The path passed to the command respects the `in-source` setting:
141+
142+
- `in-source: true` → path next to the source file (e.g., `src/Foo.js`)
143+
- `in-source: false` → path in `lib/<module>/` directory (e.g., `lib/es6/src/Foo.mjs`)
144+
145+
The command runs with the same working directory as the rewatch process (typically the project root).
146+
147+
stdout and stderr from the command are logged.
139148

140149
### Package-Spec
141150

rewatch/src/build/compile.rs

Lines changed: 39 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ use crate::project_context::ProjectContext;
1313
use ahash::{AHashMap, AHashSet};
1414
use anyhow::{Result, anyhow};
1515
use console::style;
16-
use log::{debug, trace};
16+
use log::{debug, info, trace, warn};
1717
use rayon::prelude::*;
1818
use std::path::Path;
1919
use std::path::PathBuf;
@@ -26,25 +26,38 @@ use std::time::SystemTime;
2626
fn execute_post_build_command(cmd: &str, js_file_path: &Path) -> Result<()> {
2727
let full_command = format!("{} {}", cmd, js_file_path.display());
2828

29+
debug!("Executing js-post-build: {}", full_command);
30+
2931
let output = if cfg!(target_os = "windows") {
3032
Command::new("cmd").args(["/C", &full_command]).output()
3133
} else {
3234
Command::new("sh").args(["-c", &full_command]).output()
3335
};
3436

3537
match output {
36-
Ok(output) if !output.status.success() => {
38+
Ok(output) => {
3739
let stderr = String::from_utf8_lossy(&output.stderr);
3840
let stdout = String::from_utf8_lossy(&output.stdout);
39-
Err(anyhow!(
40-
"js-post-build command failed for {}: {}{}",
41-
js_file_path.display(),
42-
stderr,
43-
stdout
44-
))
41+
42+
// Always log stdout/stderr - the user explicitly configured this command
43+
// and likely cares about its output
44+
if !stdout.is_empty() {
45+
info!("{}", stdout.trim());
46+
}
47+
if !stderr.is_empty() {
48+
warn!("{}", stderr.trim());
49+
}
50+
51+
if !output.status.success() {
52+
Err(anyhow!(
53+
"js-post-build command failed for {}",
54+
js_file_path.display()
55+
))
56+
} else {
57+
Ok(())
58+
}
4559
}
4660
Err(e) => Err(anyhow!("Failed to execute js-post-build command: {}", e)),
47-
Ok(_) => Ok(()),
4861
}
4962
}
5063

@@ -853,10 +866,23 @@ fn compile_file(
853866
{
854867
// Execute post-build command for each package spec (each output format)
855868
for spec in root_config.get_package_specs() {
856-
let js_file = helpers::get_source_file_from_rescript_file(
857-
&package.get_build_path().join(path),
858-
&root_config.get_suffix(&spec),
859-
);
869+
// Determine the correct JS file path based on in-source setting:
870+
// - in-source: true -> next to the source file (e.g., src/Foo.js)
871+
// - in-source: false -> in lib/<module>/ directory (e.g., lib/es6/src/Foo.js)
872+
let js_file = if spec.in_source {
873+
helpers::get_source_file_from_rescript_file(
874+
&Path::new(&package.path).join(path),
875+
&root_config.get_suffix(&spec),
876+
)
877+
} else {
878+
helpers::get_source_file_from_rescript_file(
879+
&Path::new(&package.path)
880+
.join("lib")
881+
.join(spec.get_out_of_source_dir())
882+
.join(path),
883+
&root_config.get_suffix(&spec),
884+
)
885+
};
860886

861887
if js_file.exists() {
862888
// Fail the build if post-build command fails (matches bsb behavior with &&)
Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,51 @@
1+
// @ts-check
2+
3+
import * as assert from "node:assert";
4+
import * as fs from "node:fs";
5+
import * as path from "node:path";
6+
import { setup } from "#dev/process";
7+
8+
const { execBuild, execClean } = setup(import.meta.dirname);
9+
10+
const isWindows = process.platform === "win32";
11+
12+
const logFile = path.join(import.meta.dirname, "post-build-paths.txt");
13+
14+
// Clean up any previous log file
15+
if (fs.existsSync(logFile)) {
16+
fs.unlinkSync(logFile);
17+
}
18+
19+
const out = await execBuild();
20+
21+
if (out.status !== 0) {
22+
assert.fail(out.stdout + out.stderr);
23+
}
24+
25+
try {
26+
// Verify that the post-build command received the correct paths
27+
assert.ok(fs.existsSync(logFile), "post-build-paths.txt should exist");
28+
29+
const paths = fs.readFileSync(logFile, "utf-8").trim().split("\n");
30+
31+
// With in-source: false and module: esmodule, the paths should be in lib/es6/
32+
// e.g., /path/to/post-build-out-of-source/lib/es6/src/demo.mjs (Unix)
33+
// e.g., C:\path\to\post-build-out-of-source\lib\es6\src\demo.mjs (Windows)
34+
const libEs6Sep = isWindows ? "\\lib\\es6\\" : "/lib/es6/";
35+
const libBsSep = isWindows ? "\\lib\\bs\\" : "/lib/bs/";
36+
37+
for (const p of paths) {
38+
assert.ok(
39+
p.includes(libEs6Sep) && p.endsWith(".mjs"),
40+
`Path should be in lib/es6/ directory with .mjs suffix: ${p}`,
41+
);
42+
// Should NOT be in lib/bs/ when in-source is false
43+
assert.ok(!p.includes(libBsSep), `Path should not be in lib/bs/: ${p}`);
44+
}
45+
} finally {
46+
// Clean up log file
47+
if (fs.existsSync(logFile)) {
48+
fs.unlinkSync(logFile);
49+
}
50+
await execClean();
51+
}
Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
import * as fs from "node:fs";
2+
import * as path from "node:path";
3+
4+
const jsFile = process.argv[2];
5+
const logFile = path.join(import.meta.dirname, "post-build-paths.txt");
6+
fs.appendFileSync(logFile, jsFile + "\n");
Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
{
2+
"name": "post-build-out-of-source",
3+
"sources": {
4+
"dir": "src",
5+
"subdirs": true
6+
},
7+
"package-specs": {
8+
"module": "esmodule",
9+
"in-source": false
10+
},
11+
"suffix": ".mjs",
12+
"js-post-build": { "cmd": "node ./log-path.js" },
13+
"warnings": {
14+
"error": "+101"
15+
}
16+
}
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
let x = 1
Lines changed: 42 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,17 +1,55 @@
11
// @ts-check
22

33
import * as assert from "node:assert";
4+
import * as fs from "node:fs";
5+
import * as path from "node:path";
46
import { setup } from "#dev/process";
57

6-
const { execBuild } = setup(import.meta.dirname);
8+
const { execBuild, execClean } = setup(import.meta.dirname);
79

8-
if (process.platform === "win32") {
9-
console.log("Skipping test on Windows");
10-
process.exit(0);
10+
const isWindows = process.platform === "win32";
11+
12+
// Use a node script for cross-platform path logging
13+
const logFile = path.join(import.meta.dirname, "post-build-paths.txt");
14+
15+
// Clean up any previous log file
16+
if (fs.existsSync(logFile)) {
17+
fs.unlinkSync(logFile);
1118
}
1219

1320
const out = await execBuild();
1421

1522
if (out.status !== 0) {
1623
assert.fail(out.stdout + out.stderr);
1724
}
25+
26+
try {
27+
// Verify that the post-build command received the correct paths
28+
assert.ok(fs.existsSync(logFile), "post-build-paths.txt should exist");
29+
30+
const paths = fs.readFileSync(logFile, "utf-8").trim().split("\n");
31+
32+
// With in-source: true, the paths should be next to the source files
33+
// e.g., /path/to/post-build/src/demo.js (Unix)
34+
// e.g., C:\path\to\post-build\src\demo.js (Windows)
35+
const srcSep = isWindows ? "\\src\\" : "/src/";
36+
const libBsSep = isWindows ? "\\lib\\bs\\" : "/lib/bs/";
37+
38+
for (const p of paths) {
39+
assert.ok(
40+
p.includes(srcSep) && p.endsWith(".js"),
41+
`Path should be in src/ directory: ${p}`,
42+
);
43+
// Should NOT be in lib/bs/ when in-source is true
44+
assert.ok(
45+
!p.includes(libBsSep),
46+
`Path should not be in lib/bs/ when in-source is true: ${p}`,
47+
);
48+
}
49+
} finally {
50+
// Clean up log file
51+
if (fs.existsSync(logFile)) {
52+
fs.unlinkSync(logFile);
53+
}
54+
await execClean();
55+
}
Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
import * as fs from "node:fs";
2+
import * as path from "node:path";
3+
4+
const jsFile = process.argv[2];
5+
const logFile = path.join(import.meta.dirname, "post-build-paths.txt");
6+
fs.appendFileSync(logFile, jsFile + "\n");

0 commit comments

Comments
 (0)