Skip to content

Commit 4092d3d

Browse files
authored
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
1 parent 2aef5b8 commit 4092d3d

File tree

12 files changed

+208
-51
lines changed

12 files changed

+208
-51
lines changed

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
- 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
1818
- `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.
1919
- Remove `external-stdlib` configuration option from `rescript.json`. This option was rarely used and is no longer supported.
20+
- `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
2021

2122
#### :eyeglasses: Spec Compliance
2223

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
},
@@ -453,7 +454,7 @@
453454
},
454455
"js-post-build": {
455456
"$ref": "#/definitions/js-post-build",
456-
"description": "(Experimental) post-processing hook. The build system will invoke `cmd ${file}` whenever a `${file}` is changed"
457+
"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)."
457458
},
458459
"package-specs": {
459460
"$ref": "#/definitions/package-specs",

rewatch/CompilerConfigurationSpec.md

Lines changed: 39 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -2,35 +2,35 @@
22

33
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.
44

5-
| Parameter | JSON type | Remark | Implemented? |
6-
| --------------------- | ----------------------- | ---------------------------------------- | :----------: |
7-
| name | string | | [x] |
8-
| namespace | boolean | | [x] |
9-
| namespace | string | | [x] |
10-
| sources | string | | [x] |
11-
| sources | array of string | | [x] |
12-
| sources | Source | | [x] |
13-
| sources | array of Source | | [x] |
14-
| ignored-dirs | array of string | | [_] |
15-
| dependencies | array of string | | [x] |
16-
| dev-dependencies | array of string | | [x] |
17-
| generators | array of Rule-Generator | | [_] |
18-
| cut-generators | boolean | | [_] |
19-
| jsx | JSX | | [x] |
20-
| gentypeconfig | Gentype | | [x] |
21-
| compiler-flags | array of string | | [x] |
22-
| warnings | Warnings | | [x] |
23-
| ppx-flags | array of string | | [x] |
24-
| pp-flags | array of string | | [_] |
25-
| js-post-build | Js-Post-Build | `${file}` is now an absolute path | [x] |
26-
| package-specs | array of Module-Format | | [_] |
27-
| package-specs | array of Package-Spec | | [x] |
28-
| entries | array of Target-Item | | [_] |
29-
| bs-external-includes | array of string | | [_] |
30-
| suffix | Suffix | | [x] |
31-
| reanalyze | Reanalyze | Reanalyze config; ignored by rewatch | [x] |
32-
| experimental-features | ExperimentalFeatures | | [x] |
33-
| editor | object | VS Code tooling only; ignored by rewatch | [x] |
5+
| Parameter | JSON type | Remark | Implemented? |
6+
| --------------------- | ----------------------- | ----------------------------------------------------------- | :----------: |
7+
| name | string | | [x] |
8+
| namespace | boolean | | [x] |
9+
| namespace | string | | [x] |
10+
| sources | string | | [x] |
11+
| sources | array of string | | [x] |
12+
| sources | Source | | [x] |
13+
| sources | array of Source | | [x] |
14+
| ignored-dirs | array of string | | [_] |
15+
| dependencies | array of string | | [x] |
16+
| dev-dependencies | array of string | | [x] |
17+
| generators | array of Rule-Generator | | [_] |
18+
| cut-generators | boolean | | [_] |
19+
| jsx | JSX | | [x] |
20+
| gentypeconfig | Gentype | | [x] |
21+
| compiler-flags | array of string | | [x] |
22+
| warnings | Warnings | | [x] |
23+
| ppx-flags | array of string | | [x] |
24+
| pp-flags | array of string | | [_] |
25+
| js-post-build | Js-Post-Build | Path respects `in-source` setting; stdout/stderr are logged | [x] |
26+
| package-specs | array of Module-Format | | [_] |
27+
| package-specs | array of Package-Spec | | [x] |
28+
| entries | array of Target-Item | | [_] |
29+
| bs-external-includes | array of string | | [_] |
30+
| suffix | Suffix | | [x] |
31+
| reanalyze | Reanalyze | Reanalyze config; ignored by rewatch | [x] |
32+
| experimental-features | ExperimentalFeatures | | [x] |
33+
| editor | object | VS Code tooling only; ignored by rewatch | [x] |
3434

3535
### Source
3636

@@ -133,7 +133,16 @@ Currently supported features:
133133

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

138147
### Package-Spec
139148

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: 40 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,20 @@
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

68
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();
@@ -16,4 +23,33 @@ if (out.status !== 0) {
1623
assert.fail(out.stdout + out.stderr);
1724
}
1825

19-
await execClean();
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)