Skip to content

Commit 1ec0a25

Browse files
jfrolichclaude
andauthored
Move rescript.json reading out of bsc into rewatch (#8365)
* Add -bs-project-root flag so bsc no longer walks for rescript.json The compiler's Ext_path.package_dir lazily walked the filesystem from cwd until it found rescript.json, solely to determine the JS output root in lam_compile_main. That walk is unnecessary when rewatch is driving the compile — rewatch already knows the package root. - compiler: add custom_package_dir ref in Ext_path; convert package_dir to a unit -> string that prefers the ref and falls back to the existing walk. Add -bs-project-root CLI flag on bsc. - rewatch: emit -bs-project-root <package-root> from compiler_args for every bsc invocation. No behavioral change for users; this is the first step toward removing direct rescript.json reads from bsc entirely. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Move gentype config from rescript.json to CLI flags The gentype backend was the last site in bsc that parsed rescript.json directly (via GenTypeConfig.read_config walking up from cwd). Rewatch already parses that file, so it can hand the needed fields to bsc as CLI flags, eliminating the filesystem walk and the duplicated JSON reading. Compiler side: - GenTypeConfig.ml: drop the JSON reader. Replace with module-level refs populated by bsc CLI flags, plus `build_config ~namespace` that assembles a Config.t from those refs. - New bsc flags: -bs-gentype-module, -bs-gentype-module-resolution, -bs-gentype-export-interfaces, -bs-gentype-generated-extension, -bs-gentype-suffix, -bs-gentype-shim, -bs-gentype-debug, -bs-gentype-dep, -bs-gentype-source-dir, -bs-gentype-bsb-project-root. -bs-project-root now also populates GenTypeConfig.project_root. - Config.t.sources becomes a `string list` (pre-expanded directories) instead of raw Ext_json_types, simplifying ModuleResolver. - Debug.set_item takes a bool-implied item name instead of an Ext_json_types.t value. - Paths.ml: drop get_config_file; read_config is now a thin wrapper around build_config. Rewatch side: - Replace `GenTypeConfig = serde_json::Value` with a typed struct; Deserialize supports both the object and (deprecated) array forms of `shims`. - New Config::get_gentype_args assembles the full -bs-gentype-* flag set from the typed gentypeconfig, suffix, dependencies, pre-expanded source dirs, and workspace root. - compile.rs: when gentype is enabled, walk the package's declared source folders (honoring `subdirs: true`) to produce the directory list gentype needs for cross-file shim resolution — `package.dirs` alone only tracks dirs with .res files. Generated .gen.tsx / .bs.js output is byte-identical; all existing gentype and full test suites pass. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Delete rescript.json filesystem walk from the OCaml compiler With rewatch always passing -bs-project-root, the fallback walk in Ext_path.find_root_filename was only reachable from direct bsc invocations. Move that walk to the Node.js wrapper (cli/bsc.js), which auto-injects -bs-project-root when absent, and make Ext_path.package_dir fail loudly if the flag was never set. - Ext_path: drop find_root_filename, find_config_dir, fallback_package_dir. package_dir () now errors out with a clear message if custom_package_dir is None. - Literals.rescript_json: removed (last reference was the deleted walk). - cli/bsc.js: walk up from cwd for rescript.json, append -bs-project-root <dir> to the args when it isn't already there. The compiler now has zero code paths that read or locate rescript.json. Locating the project root is a concern entirely for the build system (rewatch) or the JS wrapper. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Remove the custom Ext_json parser Three sites still used compiler/ext/ext_json* after the rescript.json removal: gentype's .sourcedirs.json reader, reanalyze's cmt-scan reader, and the parser's own ounit test suite. Each is handled separately so the custom lexer-based parser can be deleted outright. Gentype: - Add -bs-gentype-dep-path <name>=<path> flag. rewatch emits one per dependency, resolved through the packages map it already holds. - GenTypeConfig.Config.t gains a dep_paths Hashtbl populated from the flags. ModuleResolver.sourcedirs_to_map now reads from config.sources + config.dep_paths instead of opening <bsb_project_root>/lib/bs/ .sourcedirs.json. The missing-.sourcedirs.json warning and the read_dirs_from_config fallback are gone — rewatch supplies both pieces directly. Reanalyze: - Delete dead readSourceDirs and readDirsFromConfig (no callers). - Port readCmtScan from Ext_json_parse to jsonlib (Json.parse), which reanalyze already depends on for its rescript.json reading. - Drop `ext` from reanalyze's dune libraries. Parser: - Delete ext_json.ml/mli, ext_json_parse.mll/mli, ext_json_noloc.ml/ mli, ext_json_types.ml. - Delete the ocamllex rule in compiler/ext/dune. - Delete ounit_ext_json_tests.ml and its registration in ounit_tests_main.ml. Net: -1006 lines. No code anywhere in the repo parses JSON with the custom lexer now; jsonlib is the only JSON parser left. Generated .gen.tsx / .bs.js output is byte-identical; make test, test-gentype, and test-rewatch all pass. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Delete unused ext modules Three small modules in compiler/ext had no callers anywhere in the repo (neither in production code, nor in tests): - ext_color: unused color/ANSI helpers - ext_position: unused Lexing.position utilities - ext_string_array: unused string-array helpers Removed alongside the Ext_json cleanup. Build and tests unchanged. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Preserve package-specs.module fallback for gentype When gentypeconfig.module is omitted, the old JSON-reading path fell back to package-specs.module (object form) before defaulting to ESModule. My flag-based rewrite dropped that fallback, so a project with "package-specs": { "module": "commonjs" } but no explicit gentypeconfig.module would suddenly emit ESModule-style imports — breaking downstream TypeScript consumption. Replay the old precedence in rewatch when assembling -bs-gentype-module: 1. gentypeconfig.module if set 2. else package-specs.module when package-specs is a single object 3. else leave the flag off (bsc applies its ESModule default, same as before) Array-form package-specs still doesn't feed the fallback — that matches the old code's Obj-only pattern exactly. Added two tests covering both paths. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Preserve list-order and dep-path-order semantics from the old code Two small alignments found during a detailed behavioral review. Each only matters in a pathological collision case (two dirs / deps with the same module name), but worth getting right since this PR promises byte-identical behavior. 1. bs_dependencies / sources: old code accumulated these with a prepend into a ref and returned the ref unreversed, so iteration proceeded in reverse-input order. My new code did List.rev and flipped the order, which would swap the last-write-wins winner in ModuleResolver's file_map on collision. Drop the List.rev. 2. dep_paths: I was sorting alphabetically in rewatch. The old code iterated the .sourcedirs.json "pkgs" array in order. Remove the sort and pass the caller's order through unchanged. Not preserved (agreed with reviewer that this was a bug): the old code's namespace field was populated from the cmt filename only when rescript.json had {"namespace": true} literally. For string-valued namespaces like {"namespace": "Custom"}, the old pattern match fell through and returned None — gentype then emitted imports like ./Module pointing at files actually named Module-Custom.bs.js. The new code (always use the cmt-derived namespace) is correct. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Cache gentype source-dir walk on Package collect_gentype_source_dirs was being called from compiler_args — i.e. once per source file per build. The result is per-package and doesn't change during a build, so the redundant walks were pure waste on gentype-enabled projects with many files. Move the walk into package discovery (packages::make), cache it on a new Package.gentype_dirs field, and only compute it for packages that have a gentype_config. compiler_args now just borrows the cached slice. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Derive Deserialize for GenTypeModule* enums The manual Deserialize impls were reinventing what serde can do with #[serde(rename = "...")] on each variant — the same pattern already in use for PackageModule in this file. Drop the hand-written impls in favor of derive; serde's default error message for unknown variants ("unknown variant `foo`, expected one of ...") is good enough. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Rename Ext_path.custom_package_dir to project_root \"Custom\" was a holdover from when there were two sources for the package dir (CLI flag vs filesystem walk fallback) — \"custom\" meant \"CLI-set\" in contrast to the walked-up default. Now that the walk is gone, there's only one source and \"custom\" adds no information. Rename the ref to [project_root] so it matches the CLI flag name [-bs-project-root] directly. The getter stays [package_dir ()] since that's the established term used by [lam_compile_main] and [js_packages_info]. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Drop misplaced comment in GenTypeConfig.build_config The comment was positioned above the record literal but only applied to two fields inside it, making it read as if it documented the whole construction. The invariant it described (kept via commit history) is better tracked in git blame than inlined at the call site. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Add changelog entry for rescript.json read removal Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * Trim changelog entry Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 3389677 commit 1ec0a25

35 files changed

Lines changed: 601 additions & 1671 deletions

CHANGELOG.md

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

4141
#### :house: Internal
4242

43+
- Move `rescript.json` reading out of `bsc` into rewatch; remove the custom OCaml JSON parser. https://github.com/rescript-lang/rescript/pull/8365
44+
4345
# 13.0.0-alpha.3
4446

4547
#### :boom: Breaking Change

analysis/reanalyze/src/Paths.ml

Lines changed: 26 additions & 121 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,3 @@
1-
module StringMap = Map_string
2-
31
let rescriptJson = "rescript.json"
42

53
let readFile filename =
@@ -127,79 +125,6 @@ let handleNamespace cmt =
127125

128126
let getModuleName cmt = cmt |> handleNamespace |> Filename.basename
129127

130-
let readDirsFromConfig ~configSources =
131-
let dirs = ref [] in
132-
let root = runConfig.projectRoot in
133-
let rec processDir ~subdirs dir =
134-
let absDir =
135-
match dir = "" with
136-
| true -> root
137-
| false -> Filename.concat root dir
138-
in
139-
if Sys.file_exists absDir && Sys.is_directory absDir then (
140-
dirs := dir :: !dirs;
141-
if subdirs then
142-
absDir |> Sys.readdir
143-
|> Array.iter (fun d -> processDir ~subdirs (Filename.concat dir d)))
144-
in
145-
let rec processSourceItem (sourceItem : Ext_json_types.t) =
146-
match sourceItem with
147-
| Str {str} -> str |> processDir ~subdirs:false
148-
| Obj {map} -> (
149-
match StringMap.find_opt map "dir" with
150-
| Some (Str {str}) ->
151-
let subdirs =
152-
match StringMap.find_opt map "subdirs" with
153-
| Some (True _) -> true
154-
| Some (False _) -> false
155-
| _ -> false
156-
in
157-
str |> processDir ~subdirs
158-
| _ -> ())
159-
| Arr {content = arr} -> arr |> Array.iter processSourceItem
160-
| _ -> ()
161-
in
162-
(match configSources with
163-
| Some sourceItem -> processSourceItem sourceItem
164-
| None -> ());
165-
!dirs
166-
167-
let readSourceDirs ~configSources =
168-
let sourceDirs =
169-
["lib"; "bs"; ".sourcedirs.json"]
170-
|> List.fold_left Filename.concat runConfig.bsbProjectRoot
171-
in
172-
let dirs = ref [] in
173-
let readDirs json =
174-
match json with
175-
| Ext_json_types.Obj {map} -> (
176-
match StringMap.find_opt map "dirs" with
177-
| Some (Arr {content = arr}) ->
178-
arr
179-
|> Array.iter (fun x ->
180-
match x with
181-
| Ext_json_types.Str {str} -> dirs := str :: !dirs
182-
| _ -> ());
183-
()
184-
| _ -> ())
185-
| _ -> ()
186-
in
187-
if sourceDirs |> Sys.file_exists then
188-
let jsonOpt = sourceDirs |> Ext_json_parse.parse_json_from_file in
189-
match jsonOpt with
190-
| exception _ -> ()
191-
| json ->
192-
if runConfig.bsbProjectRoot <> runConfig.projectRoot then (
193-
readDirs json;
194-
dirs := readDirsFromConfig ~configSources)
195-
else readDirs json
196-
else (
197-
if !Cli.debug then (
198-
Log_.item "Warning: can't find source dirs: %s\n" sourceDirs;
199-
Log_.item "Types for cross-references will not be found.\n");
200-
dirs := readDirsFromConfig ~configSources);
201-
!dirs
202-
203128
type cmt_scan_entry = {
204129
build_root: string;
205130
scan_dirs: string list;
@@ -220,51 +145,31 @@ let readCmtScan () =
220145
["lib"; "bs"; ".sourcedirs.json"]
221146
|> List.fold_left Filename.concat runConfig.bsbProjectRoot
222147
in
223-
let entries = ref [] in
224-
let read_entry (json : Ext_json_types.t) =
225-
match json with
226-
| Ext_json_types.Obj {map} -> (
227-
let build_root =
228-
match StringMap.find_opt map "build_root" with
229-
| Some (Ext_json_types.Str {str}) -> Some str
230-
| _ -> None
231-
in
232-
let scan_dirs =
233-
match StringMap.find_opt map "scan_dirs" with
234-
| Some (Ext_json_types.Arr {content = arr}) ->
235-
arr |> Array.to_list
236-
|> List.filter_map (fun x ->
237-
match x with
238-
| Ext_json_types.Str {str} -> Some str
239-
| _ -> None)
240-
| _ -> []
241-
in
242-
let also_scan_build_root =
243-
match StringMap.find_opt map "also_scan_build_root" with
244-
| Some (Ext_json_types.True _) -> true
245-
| Some (Ext_json_types.False _) -> false
246-
| _ -> false
247-
in
248-
match build_root with
249-
| Some build_root ->
250-
entries := {build_root; scan_dirs; also_scan_build_root} :: !entries
251-
| None -> ())
252-
| _ -> ()
148+
let get key fn json =
149+
Json.get key json |> Option.to_list |> List.filter_map fn
253150
in
254-
let read_cmt_scan (json : Ext_json_types.t) =
255-
match json with
256-
| Ext_json_types.Obj {map} -> (
257-
match StringMap.find_opt map "cmt_scan" with
258-
| Some (Ext_json_types.Arr {content = arr}) ->
259-
arr |> Array.iter read_entry
260-
| _ -> ())
261-
| _ -> ()
151+
let read_entry (json : Json.t) =
152+
let build_root = json |> get "build_root" Json.string in
153+
let scan_dirs =
154+
match json |> get "scan_dirs" Json.array with
155+
| [arr] -> arr |> List.filter_map Json.string
156+
| _ -> []
157+
in
158+
let also_scan_build_root =
159+
match json |> get "also_scan_build_root" Json.bool with
160+
| [b] -> b
161+
| _ -> false
162+
in
163+
match build_root with
164+
| [build_root] -> Some {build_root; scan_dirs; also_scan_build_root}
165+
| _ -> None
262166
in
263-
if sourceDirsFile |> Sys.file_exists then (
264-
let jsonOpt = sourceDirsFile |> Ext_json_parse.parse_json_from_file in
265-
match jsonOpt with
266-
| exception _ -> []
267-
| json ->
268-
read_cmt_scan json;
269-
!entries |> List.rev)
270-
else []
167+
match readFile sourceDirsFile with
168+
| None -> []
169+
| Some text -> (
170+
match Json.parse text with
171+
| None -> []
172+
| Some json -> (
173+
match json |> get "cmt_scan" Json.array with
174+
| [arr] -> arr |> List.filter_map read_entry
175+
| _ -> []))

analysis/reanalyze/src/dune

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,4 +2,4 @@
22
(name reanalyze)
33
(flags
44
(-w "+6+26+27+32+33+39"))
5-
(libraries reactive jsonlib ext ml str unix))
5+
(libraries reactive jsonlib ml str unix))

cli/bsc.js

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,14 +3,40 @@
33
// @ts-check
44

55
import { execFileSync } from "node:child_process";
6+
import { existsSync } from "node:fs";
7+
import { dirname, join } from "node:path";
68

79
import { bsc_exe } from "./common/bins.js";
810
import { runtimePath } from "./common/runtime.js";
911

12+
/**
13+
* Walk up from `startDir` until a directory containing `rescript.json`
14+
* is found. Returns null if none is found.
15+
* @param {string} startDir
16+
* @returns {string | null}
17+
*/
18+
function findProjectRoot(startDir) {
19+
let dir = startDir;
20+
while (true) {
21+
if (existsSync(join(dir, "rescript.json"))) {
22+
return dir;
23+
}
24+
const parent = dirname(dir);
25+
if (parent === dir) return null;
26+
dir = parent;
27+
}
28+
}
29+
1030
const delegate_args = process.argv.slice(2);
1131
if (!delegate_args.includes("-runtime-path")) {
1232
delegate_args.push("-runtime-path", runtimePath);
1333
}
34+
if (!delegate_args.includes("-bs-project-root")) {
35+
const projectRoot = findProjectRoot(process.cwd());
36+
if (projectRoot !== null) {
37+
delegate_args.push("-bs-project-root", projectRoot);
38+
}
39+
}
1440

1541
try {
1642
execFileSync(bsc_exe, delegate_args, { stdio: "inherit" });

compiler/bsc/rescript_compiler_main.ml

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -263,6 +263,11 @@ let command_line_flags : (string * Bsc_args.spec * string) array =
263263
string_call Js_packages_state.update_npm_package_path,
264264
"*internal* Set npm-output-path: [opt_module]:path, for example: \
265265
'lib/cjs', 'amdjs:lib/amdjs', 'es6:lib/es6' " );
266+
( "-bs-project-root",
267+
string_call (fun s ->
268+
Ext_path.project_root := Some s;
269+
GenTypeConfig.project_root := s),
270+
"*internal* Set the project root directory" );
266271
( "-bs-ast",
267272
unit_call (fun _ ->
268273
Js_config.binary_ast := true;
@@ -293,6 +298,49 @@ let command_line_flags : (string * Bsc_args.spec * string) array =
293298
set Clflags.transparent_modules,
294299
"*internal*Do not record dependencies for module aliases" );
295300
("-bs-gentype", set Clflags.bs_gentype, "*internal* Pass gentype command");
301+
( "-bs-gentype-module",
302+
string_call (fun s ->
303+
GenTypeConfig.module_flag := GenTypeConfig.module_of_string s),
304+
"*internal* Set gentype module system: commonjs|esmodule" );
305+
( "-bs-gentype-module-resolution",
306+
string_call (fun s ->
307+
GenTypeConfig.module_resolution_flag :=
308+
GenTypeConfig.module_resolution_of_string s),
309+
"*internal* Set gentype module resolution strategy: node|node16|bundler"
310+
);
311+
( "-bs-gentype-export-interfaces",
312+
set GenTypeConfig.export_interfaces_flag,
313+
"*internal* Emit gentype interface files" );
314+
( "-bs-gentype-generated-extension",
315+
string_call (fun s ->
316+
GenTypeConfig.generated_file_extension_flag := Some s),
317+
"*internal* Set gentype generated-file extension (e.g. .gen.tsx)" );
318+
( "-bs-gentype-suffix",
319+
string_call (fun s -> GenTypeConfig.suffix_flag := Some s),
320+
"*internal* Set gentype import-path suffix (e.g. .bs.js, .mjs)" );
321+
( "-bs-gentype-shim",
322+
string_call GenTypeConfig.add_shim,
323+
"*internal* Register a gentype shim mapping: From=To (repeatable)" );
324+
( "-bs-gentype-debug",
325+
string_call Debug.set_item,
326+
"*internal* Enable a gentype debug category (repeatable): \
327+
all|basic|codeItems|config|converter|dependencies|moduleResolution|notImplemented|translation|typeEnv|typeResolution"
328+
);
329+
( "-bs-gentype-dep",
330+
string_call GenTypeConfig.add_bs_dependency,
331+
"*internal* Register a gentype bsb dependency (repeatable)" );
332+
( "-bs-gentype-source-dir",
333+
string_call GenTypeConfig.add_source_dir,
334+
"*internal* Register a gentype source directory relative to the project \
335+
root (repeatable)" );
336+
( "-bs-gentype-dep-path",
337+
string_call GenTypeConfig.add_dep_path,
338+
"*internal* Register a gentype dependency install path: \
339+
<name>=<absolute-path> (repeatable)" );
340+
( "-bs-gentype-bsb-project-root",
341+
string_call (fun s -> GenTypeConfig.bsb_project_root := s),
342+
"*internal* Set gentype bsb project root (workspace root containing \
343+
.sourcedirs.json)" );
296344
(******************************************************************************)
297345
( "-unboxed-types",
298346
set Clflags.unboxed_types,

compiler/core/lam_compile_main.ml

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -314,12 +314,12 @@ let lambda_as_module
314314
let basename =
315315
Ext_namespace.change_ext_ns_suffix (Filename.basename output_prefix) suffix
316316
in
317-
let target_file =
318-
(Lazy.force Ext_path.package_dir //
317+
let target_file =
318+
(Ext_path.package_dir () //
319319
path //
320320
basename
321321
(* #913 only generate little-case js file *)
322-
) in
322+
) in
323323
(if not !Clflags.dont_write_files then
324324
Ext_pervasives.with_file_as_chan
325325
target_file output_chan );

compiler/ext/dune

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,8 +15,6 @@
1515
(language c)
1616
(names ext_basic_hash_stubs)))
1717

18-
(ocamllex ext_json_parse)
19-
2018
(rule
2119
(targets hash_set_string.ml)
2220
(deps hash_set.cppo.ml)

compiler/ext/ext_color.ml

Lines changed: 0 additions & 74 deletions
This file was deleted.

0 commit comments

Comments
 (0)