diff --git a/CHANGELOG.md b/CHANGELOG.md index 0cb4958cc55..340c7e9235e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -28,6 +28,7 @@ #### :rocket: New Feature +- Add opt-in platform modules such as `Button.android.res` and `Button.ios.res`, with platform-specific JavaScript outputs sharing one `Button.resi` interface. https://github.com/rescript-lang/rescript/pull/8637 - Support UTF-16 surrogate-pair escapes such as `"\uD83D\uDE00"` in ordinary string literals. https://github.com/rescript-lang/rescript/pull/8606 - Support dynamic imports of external bindings annotated with `@scope`; the generated import follows the complete property path. These imports were previously rejected. https://github.com/rescript-lang/rescript/pull/8582 - Add `@res.hoistedFunction` for emitting nested module functions as flat JavaScript exports. https://github.com/rescript-lang/rescript/pull/8402 diff --git a/analysis/src/find_files.ml b/analysis/src/find_files.ml index 0891e210825..0e606b216d5 100644 --- a/analysis/src/find_files.ml +++ b/analysis/src/find_files.ml @@ -116,6 +116,27 @@ let get_namespace config = in from_name |> Option.map name_space_to_name +let get_platforms config = + match + config |> Yojson_helpers.get "platforms" |> bind Yojson_helpers.to_list_opt + with + | None -> [] + | Some platforms -> platforms |> List.filter_map Yojson_helpers.string_opt + +let get_platform_implementation platforms file = + if not (Filename.check_suffix file ".res") then None + else + let stem = Filename.basename file |> Filename.chop_extension in + platforms + |> List.find_map (fun platform -> + let suffix = "." ^ platform in + if Filename.check_suffix stem suffix then + Some + ( platform, + String.sub stem 0 (String.length stem - String.length suffix) + |> String.capitalize_ascii ) + else None) + module String_set = Set.Make (String) let get_public config = @@ -180,7 +201,8 @@ let find_package_root ~base ~sourcedirs_package_roots name = | _ -> Module_resolution.resolve_node_module_path ~start_path:base name (* returns a list of (absolute path to cmt(i), relative path from base to source file) *) -let find_project_files ~public ~namespace ~path ~source_directories ~lib_bs = +let find_project_files ~public ~namespace ~platforms ~path ~source_directories + ~lib_bs = let dirs = source_directories |> List.map (Filename.concat path) |> String_set.of_list in @@ -205,39 +227,77 @@ let find_project_files ~public ~namespace ~path ~source_directories ~lib_bs = let normals = files |> String_set.elements - |> Utils.filter_map (fun file -> + |> List.concat_map (fun file -> if is_implementation file then ( - let module_name = get_name file in + let platform_implementation = + get_platform_implementation platforms file + in + let physical_module_name = get_name file in + let module_name = + match platform_implementation with + | Some (_, logical_module_name) -> logical_module_name + | None -> physical_module_name + in + let is_primary_platform = + match (platforms, platform_implementation) with + | primary :: _, Some (platform, _) -> primary = platform + | _ -> true + in let resi = Hashtbl.find_opt interfaces module_name in - Hashtbl.remove interfaces module_name; + if is_primary_platform then Hashtbl.remove interfaces module_name; let base = compiled_base_name ~namespace (Files.relpath path file) in - match resi with - | Some resi -> - let cmti = (lib_bs /+ base) ^ ".cmti" in + match (platform_implementation, is_primary_platform, resi) with + | Some _, false, _ -> + let cmt = (lib_bs /+ base) ^ ".cmt" in + if Files.exists cmt then + [ + ( physical_module_name, + module_name, + Shared_types.Impl {cmt; res = file} ); + ] + else ( + Log.log ("Bad platform source file (no cmt) " ^ (lib_bs /+ base)); + []) + | _, _, Some resi -> + let interface_base = + compiled_base_name ~namespace (Files.relpath path resi) + in + let cmti = (lib_bs /+ interface_base) ^ ".cmti" in let cmt = (lib_bs /+ base) ^ ".cmt" in if Files.exists cmti then if Files.exists cmt then (* Log.log("Intf and impl " ++ cmti ++ " " ++ cmt) *) - Some + let logical_entry = ( module_name, + module_name, Shared_types.IntfAndImpl {cmti; resi; cmt; res = file} ) - else None + in + match platform_implementation with + | Some _ -> + [ + logical_entry; + ( physical_module_name, + module_name, + Shared_types.Impl {cmt; res = file} ); + ] + | None -> [logical_entry] + else [] else ( (* Log.log("Just intf " ++ cmti) *) Log.log ("Bad source file (no cmt/cmti/cmi) " ^ (lib_bs /+ base)); - None) - | None -> + []) + | _, _, None -> let cmt = (lib_bs /+ base) ^ ".cmt" in - if Files.exists cmt then Some (module_name, Impl {cmt; res = file}) + if Files.exists cmt then + [(module_name, module_name, Shared_types.Impl {cmt; res = file})] else ( Log.log ("Bad source file (no cmt/cmi) " ^ (lib_bs /+ base)); - None)) - else None) + [])) + else []) in let result = normals - |> List.filter_map (fun (name, paths) -> - let original_name = name in + |> List.filter_map (fun (name, public_name, paths) -> let name = match namespace with | None -> name @@ -245,7 +305,7 @@ let find_project_files ~public ~namespace ~path ~source_directories ~lib_bs = in match public with | Some public -> - if public |> String_set.mem original_name then Some (name, paths) + if public |> String_set.mem public_name then Some (name, paths) else None | None -> Some (name, paths)) in @@ -316,7 +376,8 @@ let find_dependency_files base config = in let project_files = find_project_files ~public:(get_public inner) ~namespace - ~path ~source_directories ~lib_bs + ~platforms:(get_platforms inner) ~path + ~source_directories ~lib_bs in Some (compiled_directories, project_files)) | None -> None diff --git a/analysis/src/packages.ml b/analysis/src/packages.ml index 1f12bf097d0..4581fa8cc36 100644 --- a/analysis/src/packages.ml +++ b/analysis/src/packages.ml @@ -106,7 +106,9 @@ let new_bs_package ~root_path = let project_files_and_paths = Find_files.find_project_files ~public:(Find_files.get_public config) - ~namespace ~path:root_path ~source_directories ~lib_bs + ~namespace + ~platforms:(Find_files.get_platforms config) + ~path:root_path ~source_directories ~lib_bs in let paths_for_module = make_paths_for_module ~project_files_and_paths diff --git a/compiler/bsc/rescript_compiler_main.ml b/compiler/bsc/rescript_compiler_main.ml index 8045973e6cf..8cdad3971a6 100644 --- a/compiler/bsc/rescript_compiler_main.ml +++ b/compiler/bsc/rescript_compiler_main.ml @@ -249,6 +249,9 @@ let command_line_flags : (string * Bsc_args.spec * string) array = ( "-bs-read-cmi", unit_call (fun _ -> Clflags.assume_no_mli := Mli_exists), "*internal* Assume mli always exist " ); + ( "-bs-platform-interface", + set Js_config.platform_interface, + "*internal* Emit platform-independent cmj metadata" ); ( "-ppx", string_list_add Clflags.all_ppx, "*internal* Pipe abstract syntax trees through preprocessor \ diff --git a/compiler/common/js_config.ml b/compiler/common/js_config.ml index 93df8b3cde6..796fe4d238a 100644 --- a/compiler/common/js_config.ml +++ b/compiler/common/js_config.ml @@ -32,6 +32,7 @@ let no_version_header = ref false let directives = ref [] let cross_module_inline = ref false +let platform_interface = ref false let debug_ir = ref false let check_lam = ref false diff --git a/compiler/common/js_config.mli b/compiler/common/js_config.mli index c4098918ca8..8d698eea355 100644 --- a/compiler/common/js_config.mli +++ b/compiler/common/js_config.mli @@ -49,6 +49,9 @@ val directives : string list ref val cross_module_inline : bool ref (** cross module inline option *) +val platform_interface : bool ref +(** emit conservative [.cmj] metadata for a platform implementation *) + val debug_ir : bool ref (** dump intermediate representations and related diagnostics *) diff --git a/compiler/core/js_packages_info.ml b/compiler/core/js_packages_info.ml index 12579018ac7..169682838ae 100644 --- a/compiler/core/js_packages_info.ml +++ b/compiler/core/js_packages_info.ml @@ -68,6 +68,13 @@ let iter (x : t) cb = Ext_list.iter x.module_systems cb let map (x : t) cb = Ext_list.map x.module_systems cb +let with_suffix suffix (x : t) = + { + x with + module_systems = + Ext_list.map x.module_systems (fun package -> {package with suffix}); + } + (* let equal (x : t) ({name; module_systems}) = x.name = name && Ext_list.for_all2_no_exn diff --git a/compiler/core/js_packages_info.mli b/compiler/core/js_packages_info.mli index 78c77b7a707..44cb456687e 100644 --- a/compiler/core/js_packages_info.mli +++ b/compiler/core/js_packages_info.mli @@ -42,6 +42,8 @@ val iter : t -> (package_info -> unit) -> unit val map : t -> (package_info -> 'a) -> 'a list +val with_suffix : string -> t -> t + val empty : t val from_name : string -> t diff --git a/compiler/core/lam_stats_export.ml b/compiler/core/lam_stats_export.ml index 7f04c5abd18..68cd7df9b4b 100644 --- a/compiler/core/lam_stats_export.ml +++ b/compiler/core/lam_stats_export.ml @@ -50,49 +50,51 @@ let values_of_export (meta : Lam_stats.t) (export_map : Lambda.t Map_ident.t) : in let persistent_closed_lambda = let optlam = Map_ident.find_opt export_map x in - match optlam with - | Some - (Lconst - ( Const_js_null | Const_js_undefined _ | Const_js_true - | Const_js_false )) - | None -> - optlam - | Some lambda -> - if not !Js_config.cross_module_inline then None - else if - Lam_analysis.safe_to_inline lambda - (* when inlning a non function, we have to be very careful, + if !Js_config.platform_interface then None + else + match optlam with + | Some + (Lconst + ( Const_js_null | Const_js_undefined _ | Const_js_true + | Const_js_false )) + | None -> + optlam + | Some lambda -> + if not !Js_config.cross_module_inline then None + else if + Lam_analysis.safe_to_inline lambda + (* when inlning a non function, we have to be very careful, only truly immutable values can be inlined *) - then - match lambda with - | Lfunction {attr = {inline = Always_inline}} - (* FIXME: is_closed lambda is too restrictive + then + match lambda with + | Lfunction {attr = {inline = Always_inline}} + (* FIXME: is_closed lambda is too restrictive It precludes ues cases - inline forEach but not forEachU *) - | Lfunction {attr = {is_a_functor = true}} -> - if Lam_closure.is_closed lambda (* TODO: seriealize more*) then - optlam - else None - | _ -> - let lam_size = Lam_analysis.size lambda in - (* TODO: + | Lfunction {attr = {is_a_functor = true}} -> + if Lam_closure.is_closed lambda (* TODO: seriealize more*) then + optlam + else None + | _ -> + let lam_size = Lam_analysis.size lambda in + (* TODO: 1. global need re-assocate when do the beta reduction 2. [lambda_exports] is not precise *) - let free_variables = - Lam_closure.free_variables Set_ident.empty Map_ident.empty - lambda - in - if - lam_size < Lam_analysis.small_inline_size - && Map_ident.is_empty free_variables - then ( - Ext_log.dwarn ~__POS__ "%s recorded for inlining @." x.name; - optlam) - else None - else None + let free_variables = + Lam_closure.free_variables Set_ident.empty Map_ident.empty + lambda + in + if + lam_size < Lam_analysis.small_inline_size + && Map_ident.is_empty free_variables + then ( + Ext_log.dwarn ~__POS__ "%s recorded for inlining @." x.name; + optlam) + else None + else None in match (arity, persistent_closed_lambda) with | Single Arity_na, (None | Some (Lconst Const_module_alias)) -> acc @@ -129,10 +131,17 @@ let get_dependent_module_effect (maybe_pure : string option) let export_to_cmj (meta : Lam_stats.t) effect_ export_map hoisted_exports case : Js_cmj_format.t = let values = values_of_export meta export_map in - - Js_cmj_format.make ~values ~hoisted_exports ~effect_ - ~package_spec:(Js_packages_state.get_packages_info ()) - ~case + let values, hoisted_exports, effect_, package_spec = + if !Js_config.platform_interface then + ( Map_string.empty, + [], + Some "platform implementation", + Js_packages_state.get_packages_info () + |> Js_packages_info.with_suffix "" ) + else + (values, hoisted_exports, effect_, Js_packages_state.get_packages_info ()) + in + Js_cmj_format.make ~values ~hoisted_exports ~effect_ ~package_spec ~case (* FIXME: make sure [-o] would not change its case add test for ns/non-ns *) diff --git a/docs/docson/build-schema.json b/docs/docson/build-schema.json index 2c3e1b30fec..a7f579e0ab1 100644 --- a/docs/docson/build-schema.json +++ b/docs/docson/build-schema.json @@ -493,6 +493,16 @@ "suffix": { "$ref": "#/definitions/suffix-spec" }, + "platforms": { + "type": "array", + "minItems": 1, + "uniqueItems": true, + "items": { + "type": "string", + "pattern": "^[a-z][a-z0-9_]*$" + }, + "description": "Platform suffixes for shared-interface modules such as Button.android.res and Button.ios.res." + }, "reanalyze": { "$ref": "#/definitions/reanalyze", "description": "Configure reanalyze, a static code analysis tool for ReScript." diff --git a/rewatch/CompilerConfigurationSpec.md b/rewatch/CompilerConfigurationSpec.md index 13af9204d94..6f03bff20cd 100644 --- a/rewatch/CompilerConfigurationSpec.md +++ b/rewatch/CompilerConfigurationSpec.md @@ -35,10 +35,39 @@ recommended for new projects. | entries | array of Target-Item | | [_] | | bs-external-includes | array of string | | [_] | | suffix | Suffix | | [x] | +| platforms | array of string | Shared-interface platform modules; see below | [x] | | reanalyze | Reanalyze | Reanalyze config; ignored by rewatch | [x] | | experimental-features | ExperimentalFeatures | | [x] | | editor | object | VS Code tooling only; ignored by rewatch | [x] | +### Platform modules + +`platforms` enables platform-specific implementations behind one ordinary +interface. The first entry is the primary implementation used for compiler +artifacts and editor navigation; generated JavaScript still contains every +configured variant. + +```json +{"platforms": ["android", "ios"]} +``` + +For a module named `Button`, the source layout is: + +```text +Button.resi +Button.android.res +Button.ios.res +``` + +Every configured implementation is required and checked against `Button.resi`. +The outputs are `Button.android.js` and `Button.ios.js` (using the configured JS +suffix), while ordinary consumers emit an extensionless import of `Button` so a +platform-aware resolver such as Metro can select the implementation. Generic +fallbacks, `.native.res`, and platform-specific interfaces are not supported. + +Platform names must begin with a lowercase ASCII letter and contain only +lowercase letters, digits, and underscores. + ### Source | Parameter | JSON type | Remark | Implemented? | diff --git a/rewatch/README.md b/rewatch/README.md index 1457fc1d074..f0139e17c23 100644 --- a/rewatch/README.md +++ b/rewatch/README.md @@ -25,6 +25,7 @@ Focused documentation: - [monorepo discovery and build scope](MonorepoSupport.md) - [feature-gated source directories](Features.md) - [integration-test workspace](testrepo/README.md) +- [platform module acceptance tests](tests/platforms/README.md) The ReScript website owns user-facing configuration documentation. The support matrix in this directory records what the current Rewatch implementation diff --git a/rewatch/src/build.rs b/rewatch/src/build.rs index 1c02d28da60..564a6f997dc 100644 --- a/rewatch/src/build.rs +++ b/rewatch/src/build.rs @@ -125,7 +125,30 @@ pub fn get_compiler_args(rescript_file_path: &Path) -> Result { /* warn_error_override */ None, )?; let is_interface = filename.to_string_lossy().ends_with('i'); - let has_interface = if is_interface { + let platform = project_context + .current_config + .platforms + .as_deref() + .and_then(|platforms| helpers::platform_implementation(relative_filename, platforms)) + .map(|(name, logical_path)| PlatformImplementation { + logical_module_name: helpers::file_path_to_module_name( + &logical_path, + &project_context.current_config.get_namespace(), + ), + primary: project_context + .current_config + .platforms + .as_ref() + .and_then(|platforms| platforms.first()) + == Some(&name), + name, + logical_path, + }); + let has_interface = if let Some(platform) = &platform { + current_package + .join(platform.logical_path.with_extension("resi")) + .exists() + } else if is_interface { true } else { let mut interface_filename = filename.to_string_lossy().to_string(); @@ -145,6 +168,7 @@ pub fn get_compiler_args(rescript_file_path: &Path) -> Result { None, // No warn_error_override for compiler-args command SourceMapCommand::Build, &[], // Source dirs not available outside full build; gentype falls back to defaults. + platform.as_ref(), )?; let result = serde_json::to_string_pretty(&CompilerArgs { @@ -207,6 +231,7 @@ pub fn initialize_build( source_map_command, ); packages::parse_packages(&mut build_state)?; + clean::reconcile_platform_outputs(&build_state); let compile_assets_state = read_compile_state::read(&mut build_state)?; @@ -434,16 +459,21 @@ pub fn incremental_build_without_lock( .unwrap(), ); - let (compile_errors, compile_warnings, num_compiled_modules) = compile::compile( + let compile_result = compile::compile( build_state, show_progress, || pb.inc(1), |size| pb.set_length(size), - ) - .map_err(|e| IncrementalBuildError { - kind: IncrementalBuildErrorKind::CompileError(Some(e.to_string())), - plain_output, - })?; + ); + // Compilation may emit some platform files before another module fails. + // Persist the current output set for the next build's stale-file cleanup + // regardless of this build's outcome. + clean::write_platform_outputs(build_state); + let (compile_errors, compile_warnings, num_compiled_modules) = + compile_result.map_err(|e| IncrementalBuildError { + kind: IncrementalBuildErrorKind::CompileError(Some(e.to_string())), + plain_output, + })?; let compile_duration = start_compiling.elapsed(); diff --git a/rewatch/src/build/build_types.rs b/rewatch/src/build/build_types.rs index 3916c6d8ef7..528dec2d7fe 100644 --- a/rewatch/src/build/build_types.rs +++ b/rewatch/src/build/build_types.rs @@ -33,9 +33,18 @@ pub struct Interface { pub compile_warnings: Option, } +#[derive(Debug, Clone, PartialEq)] +pub struct PlatformImplementation { + pub name: String, + pub logical_path: PathBuf, + pub logical_module_name: String, + pub primary: bool, +} + #[derive(Debug, Clone, PartialEq)] pub struct Implementation { pub path: PathBuf, + pub platform: Option>, pub parse_state: ParseState, pub compile_state: CompileState, pub last_modified: SystemTime, diff --git a/rewatch/src/build/clean.rs b/rewatch/src/build/clean.rs index 93cdcfeabba..28c334b47b8 100644 --- a/rewatch/src/build/clean.rs +++ b/rewatch/src/build/clean.rs @@ -10,6 +10,7 @@ use anyhow::Result; use console::style; use rayon::prelude::*; use std::io::Write; +use std::path::Component; use std::path::{Path, PathBuf}; use std::time::Instant; use tracing::instrument; @@ -103,6 +104,108 @@ fn clean_source_files(packages: &AHashMap, root_config: &Config .for_each(|(rescript_file_location, suffix)| remove_mjs_file(rescript_file_location, suffix)); } +const PLATFORM_OUTPUTS_FILE: &str = ".platform-outputs.json"; + +fn platform_outputs_path(package: &Package) -> PathBuf { + package.path.join("lib").join(PLATFORM_OUTPUTS_FILE) +} + +fn is_safe_relative_path(path: &Path) -> bool { + !path.is_absolute() + && path.components().all(|component| { + !matches!( + component, + Component::ParentDir | Component::RootDir | Component::Prefix(_) + ) + }) +} + +fn read_recorded_platform_outputs(package: &Package) -> Vec { + std::fs::read_to_string(platform_outputs_path(package)) + .ok() + .and_then(|contents| serde_json::from_str::>(&contents).ok()) + .unwrap_or_default() + .into_iter() + .filter(|path| is_safe_relative_path(path)) + .collect() +} + +fn expected_platform_outputs(build_state: &BuildState, package: &Package) -> AHashSet { + let root_config = build_state.get_root_config(); + build_state + .modules + .values() + .filter(|module| module.package_name == package.name) + .filter_map(|module| match &module.source_type { + SourceType::SourceFile(source_file) => source_file.implementation.platform.as_ref(), + SourceType::MlMap(_) => None, + }) + .flat_map(|platform| { + root_config.get_package_specs().into_iter().map(move |spec| { + let output_dir = if spec.in_source { + platform.logical_path.parent().unwrap().to_path_buf() + } else { + Path::new("lib") + .join(spec.get_out_of_source_dir()) + .join(platform.logical_path.parent().unwrap()) + }; + let basename = platform.logical_path.file_stem().unwrap().to_string_lossy(); + output_dir.join(format!( + "{basename}.{}{}", + platform.name, + root_config.get_suffix(&spec) + )) + }) + }) + .collect() +} + +pub fn reconcile_platform_outputs(build_state: &BuildState) { + build_state.packages.values().for_each(|package| { + let expected = expected_platform_outputs(build_state, package); + for previous in read_recorded_platform_outputs(package) { + if !expected.contains(&previous) { + let output = package.path.join(previous); + let _ = std::fs::remove_file(&output); + let _ = std::fs::remove_file(PathBuf::from(format!("{}.map", output.to_string_lossy()))); + } + } + }); +} + +pub fn write_platform_outputs(build_state: &BuildState) { + build_state.packages.values().for_each(|package| { + let mut outputs = expected_platform_outputs(build_state, package) + .into_iter() + .collect::>(); + outputs.sort(); + let manifest = platform_outputs_path(package); + if outputs.is_empty() { + let _ = std::fs::remove_file(manifest); + return; + } + let Ok(contents) = serde_json::to_string(&outputs) else { + return; + }; + if std::fs::read_to_string(&manifest).ok().as_deref() == Some(&contents) { + return; + } + if let Some(parent) = manifest.parent() { + let _ = std::fs::create_dir_all(parent); + } + let _ = std::fs::write(manifest, contents); + }); +} + +pub fn remove_recorded_platform_outputs(package: &Package) { + for relative in read_recorded_platform_outputs(package) { + let output = package.path.join(relative); + let _ = std::fs::remove_file(&output); + let _ = std::fs::remove_file(PathBuf::from(format!("{}.map", output.to_string_lossy()))); + } + let _ = std::fs::remove_file(platform_outputs_path(package)); +} + // TODO: change to scan_previous_build => CompileAssetsState // and then do cleanup on that state (for instance remove all .mjs files that are not in the state) @@ -163,6 +266,7 @@ pub fn cleanup_previous_build( .for_each(|res_file_location| { let AstModule { module_name, + package_name, last_modified: ast_last_modified, ast_file_path, .. @@ -170,6 +274,27 @@ pub fn cleanup_previous_build( .ast_modules .get(res_file_location) .expect("Could not find module name for ast file"); + if helpers::is_interface_ast_file(ast_file_path) { + let package_path = build_state.packages.get(package_name).unwrap().path.clone(); + for module in build_state + .modules + .values_mut() + .filter(|module| module.package_name == *package_name) + { + let SourceType::SourceFile(source_file) = &mut module.source_type else { + continue; + }; + let Some(interface) = &mut source_file.interface else { + continue; + }; + if package_path.join(&interface.path) == *res_file_location + && ast_last_modified > &interface.last_modified + { + interface.parse_dirty = false; + } + } + return; + } let module = build_state .modules .get_mut(module_name) @@ -190,24 +315,11 @@ pub fn cleanup_previous_build( match &mut module.source_type { SourceType::MlMap(_) => unreachable!("MlMap is not matched with a ReScript file"), SourceType::SourceFile(source_file) => { - if helpers::is_interface_ast_file(ast_file_path) { - let interface = source_file - .interface - .as_mut() - .expect("Could not find interface for module"); - - let source_last_modified = interface.last_modified; - if ast_last_modified > &source_last_modified { - interface.parse_dirty = false; - } - } else { - let implementation = &mut source_file.implementation; - let source_last_modified = implementation.last_modified; - if ast_last_modified > &source_last_modified - && !deleted_interfaces.contains(module_name) - { - implementation.parse_dirty = false; - } + let implementation = &mut source_file.implementation; + let source_last_modified = implementation.last_modified; + if ast_last_modified > &source_last_modified && !deleted_interfaces.contains(module_name) + { + implementation.parse_dirty = false; } } } @@ -356,6 +468,7 @@ pub fn clean(path: &Path, show_progress: bool, plain_output: bool, prod: bool) - }; for (_, package) in &packages { + remove_recorded_platform_outputs(package); clean_package(show_progress, plain_output, package) } diff --git a/rewatch/src/build/compile.rs b/rewatch/src/build/compile.rs index 6a0868dd507..b624254b343 100644 --- a/rewatch/src/build/compile.rs +++ b/rewatch/src/build/compile.rs @@ -248,15 +248,24 @@ fn compile_one( tracing::Span::none().entered() }; - let cmi_path = helpers::get_compiler_asset( - package, - &package.namespace, - &source_file.implementation.path, - "cmi", - ); + let cmi_source_path = source_file + .implementation + .platform + .as_ref() + .map(|platform| platform.logical_path.as_path()) + .unwrap_or(&source_file.implementation.path); + let cmi_path = helpers::get_compiler_asset(package, &package.namespace, cmi_source_path, "cmi"); let cmi_digest = helpers::compute_file_hash(Path::new(&cmi_path)); + let should_compile_interface = source_file + .implementation + .platform + .as_ref() + .is_none_or(|platform| platform.primary); let interface_result = source_file.interface.as_ref().map(|iface| { + if !should_compile_interface { + return Ok(None); + } compile_file( package, &helpers::get_ast_path(&iface.path), @@ -763,10 +772,14 @@ pub fn compiler_args( // Pre-expanded source directories for the current package (used by gentype). // Pass an empty slice when unavailable (e.g. the compiler-args CLI command). current_package_dirs: &[PathBuf], + platform: Option<&PlatformImplementation>, ) -> Result> { let bsc_flags = config::flatten_flags(&config.compiler_flags); let dependency_paths = get_dependency_paths(config, project_context, packages, is_type_dev); - let module_name = helpers::file_path_to_module_name(file_path, &config.get_namespace()); + let module_path = platform + .map(|platform| platform.logical_path.as_path()) + .unwrap_or(file_path); + let module_name = helpers::file_path_to_module_name(module_path, &config.get_namespace()); let namespace_args = match &config.get_namespace() { packages::Namespace::NamespaceWithEntry { namespace: _, entry } if &module_name == entry => { @@ -812,7 +825,19 @@ pub fn compiler_args( } else { Vec::new() }; - let gentype_arg = config.get_gentype_args(current_package_dirs, Some(bsb_project_root), &dep_paths); + // Every platform implementation shares one interface and therefore one + // GenType wrapper. Let the primary implementation generate it so separate + // compiler processes do not repeatedly read and write the same output. + let gentype_arg = if platform.is_some_and(|platform| !platform.primary) { + vec![] + } else { + config.get_gentype_args( + current_package_dirs, + Some(bsb_project_root), + &dep_paths, + platform.map(|_| ""), + ) + }; let experimental_args = root_config.get_experimental_features_args(); let warning_args = config.get_warning_args(is_local_dep, warn_error_override); @@ -857,7 +882,9 @@ pub fn compiler_args( .unwrap() .to_string() }, - root_config.get_suffix(spec), + platform + .map(|platform| format!(".{}{}", platform.name, root_config.get_suffix(spec))) + .unwrap_or_else(|| root_config.get_suffix(spec)), ), ] }) @@ -866,6 +893,30 @@ pub fn compiler_args( let runtime_path_args = get_runtime_path_args(config, project_context)?; + let platform_args = if is_interface { + vec![] + } else { + platform + .map(|platform| { + vec![ + "-bs-platform-interface".to_string(), + "-o".to_string(), + Path::new("__platform") + .join(&platform.name) + .join(format!( + "{}.cmj", + helpers::file_path_to_compiler_asset_basename( + &platform.logical_path, + &config.get_namespace() + ) + )) + .to_string_lossy() + .to_string(), + ] + }) + .unwrap_or_default() + }; + Ok(vec![ namespace_args, read_cmi_args, @@ -892,6 +943,7 @@ pub fn compiler_args( package_name_arg, project_root_args, implementation_args, + platform_args, // vec![ // "-I".to_string(), // abs_node_modules_path.to_string() + "/rescript/ocaml", @@ -1011,6 +1063,38 @@ fn compile_file( .map_err(|e| anyhow!(e))?; let basename = helpers::file_path_to_compiler_asset_basename(implementation_file_path, &package.namespace); + let platform = match &module.source_type { + SourceType::SourceFile(source_file) => source_file.implementation.platform.as_deref(), + SourceType::MlMap(_) => None, + }; + let logical_basename = platform + .map(|platform| { + helpers::file_path_to_compiler_asset_basename(&platform.logical_path, &package.namespace) + }) + .unwrap_or_else(|| basename.clone()); + if !is_interface && let Some(platform) = platform { + let platform_dir = build_path_abs.join("__platform").join(&platform.name); + helpers::create_path(&platform_dir); + if package.config.gentype_config.is_some() + && platform.primary + && let Some(interface) = module.get_interface().as_ref() + { + let interface_dir = interface.path.parent().unwrap(); + let shared_cmti = build_path_abs + .join(interface_dir) + .join(format!("{logical_basename}.cmti")); + std::fs::copy( + &shared_cmti, + platform_dir.join(format!("{logical_basename}.cmti")), + ) + .map_err(|error| { + anyhow!( + "Could not prepare shared interface '{}' for GenType: {error}", + shared_cmti.display() + ) + })?; + } + } let has_interface = module.get_interface().is_some(); let is_type_dev = module.is_type_dev; // `gentype_dirs` is populated once during package discovery, so we just @@ -1029,6 +1113,7 @@ fn compile_file( warn_error_override, build_state.source_map_command, current_package_dirs, + platform, )?; let to_mjs = Command::new(&compiler_info.bsc_path) @@ -1055,44 +1140,78 @@ fn compile_file( let err = compiler_output_to_string(&x.stderr); let dir = Path::new(implementation_file_path).parent().unwrap(); - + let platform_info = match &module.source_type { + SourceType::SourceFile(source_file) => source_file.implementation.platform.as_ref(), + SourceType::MlMap(_) => None, + }; // perhaps we can do this copying somewhere else if !is_interface { + if let Some(platform) = platform_info { + let platform_dir = package.get_build_path().join("__platform").join(&platform.name); + let platform_cmj = platform_dir.join(format!("{logical_basename}.cmj")); + let platform_cmt = platform_dir.join(format!("{logical_basename}.cmt")); + let _ = std::fs::copy( + &platform_cmj, + ocaml_build_path_abs.join(format!("{logical_basename}.{}.cmj", platform.name)), + ); + let _ = std::fs::copy( + &platform_cmt, + ocaml_build_path_abs.join(format!("{logical_basename}.{}.cmt", platform.name)), + ); + let physical_basename = helpers::file_path_to_compiler_asset_basename( + implementation_file_path, + &package.namespace, + ); + let _ = std::fs::copy( + &platform_cmt, + package + .get_build_path() + .join(dir) + .join(format!("{physical_basename}.cmt")), + ); + if platform.primary { + let _ = std::fs::copy( + platform_cmj, + ocaml_build_path_abs.join(format!("{logical_basename}.cmj")), + ); + let _ = std::fs::copy( + &platform_cmt, + ocaml_build_path_abs.join(format!("{logical_basename}.cmt")), + ); + } + } else { + let _ = std::fs::copy( + package.get_build_path().join(dir).join(format!("{basename}.cmi")), + ocaml_build_path_abs.join(format!("{basename}.cmi")), + ); + let _ = std::fs::copy( + package.get_build_path().join(dir).join(format!("{basename}.cmj")), + ocaml_build_path_abs.join(format!("{basename}.cmj")), + ); + let _ = std::fs::copy( + package.get_build_path().join(dir).join(format!("{basename}.cmt")), + ocaml_build_path_abs.join(format!("{basename}.cmt")), + ); + } + } else { + let interface_dir = module + .get_interface() + .as_ref() + .and_then(|interface| interface.path.parent()) + .unwrap_or(dir); let _ = std::fs::copy( package .get_build_path() - .join(dir) - // because editor tooling doesn't support namespace entries yet - // we just remove the @ for now. This makes sure the editor support - // doesn't break - .join(format!("{basename}.cmi")), - ocaml_build_path_abs.join(format!("{basename}.cmi")), - ); - let _ = std::fs::copy( - package.get_build_path().join(dir).join(format!("{basename}.cmj")), - ocaml_build_path_abs.join(format!("{basename}.cmj")), - ); - let _ = std::fs::copy( - package - .get_build_path() - .join(dir) - // because editor tooling doesn't support namespace entries yet - // we just remove the @ for now. This makes sure the editor support - // doesn't break - .join(format!("{basename}.cmt")), - ocaml_build_path_abs.join(format!("{basename}.cmt")), + .join(interface_dir) + .join(format!("{logical_basename}.cmti")), + ocaml_build_path_abs.join(format!("{logical_basename}.cmti")), ); - } else { let _ = std::fs::copy( package .get_build_path() - .join(dir) - .join(format!("{basename}.cmti")), - ocaml_build_path_abs.join(format!("{basename}.cmti")), - ); - let _ = std::fs::copy( - package.get_build_path().join(dir).join(format!("{basename}.cmi")), - ocaml_build_path_abs.join(format!("{basename}.cmi")), + .join(interface_dir) + .join(format!("{logical_basename}.cmi")), + ocaml_build_path_abs.join(format!("{logical_basename}.cmi")), ); } @@ -1425,6 +1544,7 @@ mod tests { source_type: SourceType::SourceFile(SourceFile { implementation: Implementation { path: PathBuf::from("src/ModuleA.res"), + platform: None, parse_state: ParseState::Success, compile_state: if implementation_warning.is_some() { CompileState::Warning diff --git a/rewatch/src/build/deps.rs b/rewatch/src/build/deps.rs index b80fc4c99a1..51e3aeebc56 100644 --- a/rewatch/src/build/deps.rs +++ b/rewatch/src/build/deps.rs @@ -127,6 +127,11 @@ pub fn get_deps(build_state: &mut BuildState, deleted_modules: &AHashSet build_state, )) } + if let Some(platform) = &source_file.implementation.platform + && !platform.primary + { + deps.insert(platform.logical_module_name.clone()); + } match &package.namespace { packages::Namespace::NamespaceWithEntry { namespace: _, entry } if entry == module_name => diff --git a/rewatch/src/build/packages.rs b/rewatch/src/build/packages.rs index 958b600b93b..d0de9b3bc44 100644 --- a/rewatch/src/build/packages.rs +++ b/rewatch/src/build/packages.rs @@ -676,10 +676,16 @@ fn extend_with_children( .into_iter() .for_each(|source| map.extend(source)); - let mut modules = AHashSet::from_iter( - map.keys() - .map(|key| helpers::file_path_to_module_name(key, &package.namespace)), - ); + let mut modules = AHashSet::from_iter(map.keys().map(|key| { + let logical_path = package + .config + .platforms + .as_deref() + .and_then(|platforms| helpers::platform_implementation(key, platforms)) + .map(|(_, logical_path)| logical_path) + .unwrap_or_else(|| key.to_path_buf()); + helpers::file_path_to_module_name(&logical_path, &package.namespace) + })); match package.namespace.to_owned() { Namespace::Namespace(namespace) => { let _ = modules.insert(namespace); @@ -990,11 +996,77 @@ pub fn parse_packages(build_state: &mut BuildState) -> Result<()> { debug!("Building source file-tree for package: {}", package.name); if let Some(source_files) = &package.source_files { - for (file, metadata) in source_files.iter() { + let platforms = package.config.platforms.as_deref().unwrap_or(&[]); + let primary_platform = platforms.first().map(String::as_str); + + for file in source_files.keys() { + if let Some((platform, common_interface)) = helpers::platform_interface(file, platforms) { + return Err(anyhow!( + "Platform-specific interface '{}' is not supported. Use the shared interface '{}' for the '{}' implementation.", + file.display(), + common_interface.display(), + platform + )); + } + } + + for file in source_files + .keys() + .filter(|file| helpers::platform_implementation(file, platforms).is_some()) + { + let (_, logical_path) = helpers::platform_implementation(file, platforms).unwrap(); + let interface_path = logical_path.with_extension("resi"); + if !source_files.contains_key(&interface_path) { + return Err(anyhow!( + "Platform module '{}' requires a shared interface '{}'.", + file.display(), + interface_path.display() + )); + } + if source_files.contains_key(&logical_path) { + return Err(anyhow!( + "Platform module family '{}' cannot also contain '{}'. Generic fallbacks are not supported yet.", + helpers::file_path_to_module_name(&logical_path, &package.namespace), + logical_path.display() + )); + } + for platform in platforms { + let expected = logical_path.with_file_name(format!( + "{}.{}.res", + logical_path.file_stem().unwrap().to_string_lossy(), + platform + )); + if !source_files.contains_key(&expected) { + return Err(anyhow!( + "Platform module '{}' is missing implementation '{}'.", + helpers::file_path_to_module_name(&logical_path, &package.namespace), + expected.display() + )); + } + } + } + + let mut files = source_files.iter().collect::>(); + // Platform interfaces must be attached after all their implementation + // nodes exist. Ordinary source pairing remains order-independent. + files.sort_by_key(|(file, _)| { + helpers::is_interface_file(file.extension().unwrap().to_str().unwrap()) + }); + for (file, metadata) in files { let namespace = package.namespace.to_owned(); let extension = file.extension().unwrap().to_str().unwrap(); - let module_name = helpers::file_path_to_module_name(file, &namespace); + let platform_implementation = helpers::platform_implementation(file, platforms); + let logical_module_name = platform_implementation + .as_ref() + .map(|(_, logical_path)| helpers::file_path_to_module_name(logical_path, &namespace)); + let module_name = match (&platform_implementation, &logical_module_name) { + (Some((platform, _)), Some(logical)) if Some(platform.as_str()) != primary_platform => { + format!("{logical}$$platform${platform}") + } + (_, Some(logical)) => logical.clone(), + _ => helpers::file_path_to_module_name(file, &namespace), + }; if helpers::is_implementation_file(extension) { // Store duplicate paths in an Option so we can build the error after the entry borrow ends. @@ -1020,6 +1092,16 @@ pub fn parse_packages(build_state: &mut BuildState) -> Result<()> { source_type: SourceType::SourceFile(SourceFile { implementation: Implementation { path: file.to_owned(), + platform: platform_implementation.as_ref().map( + |(platform, logical_path)| { + Box::new(PlatformImplementation { + name: platform.clone(), + logical_path: logical_path.clone(), + logical_module_name: logical_module_name.clone().unwrap(), + primary: Some(platform.as_str()) == primary_platform, + }) + }, + ), parse_state: ParseState::Pending, compile_state: CompileState::Pending, last_modified: metadata.modified, @@ -1064,6 +1146,40 @@ pub fn parse_packages(build_state: &mut BuildState) -> Result<()> { }; match source_files.get(&implementation_filename) { None => { + let platform_nodes = build_state + .modules + .iter() + .filter_map(|(name, module)| match &module.source_type { + SourceType::SourceFile(source_file) + if module.package_name == package.name + && source_file.implementation.platform.as_ref().is_some_and( + |platform| platform.logical_path == implementation_filename, + ) => + { + Some(name.clone()) + } + _ => None, + }) + .collect::>(); + if !platform_nodes.is_empty() { + for name in platform_nodes { + if let Some(Module { + source_type: SourceType::SourceFile(source_file), + .. + }) = build_state.modules.get_mut(&name) + { + source_file.interface = Some(Interface { + path: file.to_owned(), + parse_state: ParseState::Pending, + compile_state: CompileState::Pending, + last_modified: metadata.modified, + parse_dirty: true, + compile_warnings: None, + }); + } + } + continue; + } if let Some(implementation_path) = source_files.keys().find(|path| { let extension = path.extension().and_then(|ext| ext.to_str()); matches!(extension, Some(ext) if helpers::is_implementation_file(ext)) @@ -1106,6 +1222,7 @@ pub fn parse_packages(build_state: &mut BuildState) -> Result<()> { // this will be overwritten later implementation: Implementation { path: implementation_filename, + platform: None, parse_state: ParseState::Pending, compile_state: CompileState::Pending, last_modified: metadata.modified, @@ -1247,14 +1364,18 @@ pub fn validate_packages_dependencies(packages: &AHashMap) -> b #[cfg(test)] mod test { + use crate::build::build_types::{BuildState, CompilerInfo, SourceType}; use crate::config; use crate::project_context::{MonoRepoContext, ProjectContext}; - use super::{Namespace, Package, read_issue_tracker_url, read_package_name}; + use super::{ + Namespace, Package, SourceFileMeta, parse_packages, read_issue_tracker_url, read_package_name, + }; use ahash::{AHashMap, AHashSet}; use std::fs; use std::path::PathBuf; use std::sync::RwLock; + use std::time::{Duration, SystemTime}; use tempfile::TempDir; pub struct CreatePackageArgs { @@ -1285,6 +1406,101 @@ mod test { is_local_dep: false, } } + + fn platform_package( + root: &std::path::Path, + name: &str, + namespace: &str, + modified: SystemTime, + is_root: bool, + ) -> Package { + let path = root.join(name); + fs::create_dir_all(path.join("src")).expect("package source directory should be created"); + let mut config = config::tests::create_config(config::tests::CreateConfigArgs { + name: name.to_string(), + bs_deps: vec![], + build_dev_deps: vec![], + allowed_dependents: None, + path: path.join("rescript.json"), + }); + config.platforms = Some(vec!["android".to_string(), "ios".to_string()]); + let source_files = ["Button.android.res", "Button.ios.res", "Button.resi"] + .into_iter() + .map(|file| { + ( + PathBuf::from("src").join(file), + SourceFileMeta { + modified, + is_type_dev: false, + }, + ) + }) + .collect(); + Package { + name: name.to_string(), + config, + source_folders: AHashSet::new(), + source_files: Some(source_files), + namespace: Namespace::Namespace(namespace.to_string()), + modules: None, + path, + dirs: None, + gentype_dirs: None, + is_local_dep: true, + is_root, + } + } + + #[test] + fn platform_interfaces_only_attach_to_modules_in_their_package() { + let temp_dir = TempDir::new().expect("temp dir should be created"); + let first_modified = SystemTime::UNIX_EPOCH + Duration::from_secs(1); + let second_modified = SystemTime::UNIX_EPOCH + Duration::from_secs(2); + let first = platform_package(temp_dir.path(), "first", "First", first_modified, true); + let second = platform_package(temp_dir.path(), "second", "Second", second_modified, false); + let current_config = first.config.clone(); + let mut packages = AHashMap::new(); + packages.insert(first.name.clone(), first); + packages.insert(second.name.clone(), second); + let project_context = ProjectContext { + current_config, + monorepo_context: None, + node_modules_exist_cache: RwLock::new(AHashMap::new()), + packages_cache: RwLock::new(AHashMap::new()), + }; + let compiler = CompilerInfo { + bsc_path: temp_dir.path().join("bsc"), + bsc_hash: blake3::hash(b"test-bsc"), + runtime_path: temp_dir.path().join("runtime"), + }; + let mut build_state = BuildState::new( + project_context, + packages, + compiler, + config::SourceMapCommand::Build, + ); + + parse_packages(&mut build_state).expect("packages should parse"); + + for (package_name, expected_modified) in [("first", first_modified), ("second", second_modified)] { + let interfaces = build_state + .modules + .values() + .filter(|module| module.package_name == package_name) + .filter_map(|module| match &module.source_type { + SourceType::SourceFile(source_file) => source_file.interface.as_ref(), + SourceType::MlMap(_) => None, + }) + .collect::>(); + assert_eq!(interfaces.len(), 2); + assert!( + interfaces + .iter() + .all(|interface| interface.last_modified == expected_modified) + ); + } + } + #[test] fn should_return_false_with_invalid_parents_as_bs_dependencies() { let mut packages: AHashMap = AHashMap::new(); diff --git a/rewatch/src/build/parse.rs b/rewatch/src/build/parse.rs index 9d4e71533af..dd1d0526d22 100644 --- a/rewatch/src/build/parse.rs +++ b/rewatch/src/build/parse.rs @@ -79,7 +79,15 @@ pub fn generate_asts( ) .map_err(|e| e.to_string()); + let should_parse_interface = source_file + .implementation + .platform + .as_ref() + .is_none_or(|platform| platform.primary); let iast_result = match source_file.interface.as_ref().map(|i| i.path.to_owned()) { + Some(interface_file_path) if !should_parse_interface => { + Ok(Some((helpers::get_ast_path(&interface_file_path), None))) + } Some(interface_file_path) => { match generate_ast( package.to_owned(), diff --git a/rewatch/src/build/read_compile_state.rs b/rewatch/src/build/read_compile_state.rs index 3b766294712..fdb4b5af098 100644 --- a/rewatch/src/build/read_compile_state.rs +++ b/rewatch/src/build/read_compile_state.rs @@ -13,6 +13,43 @@ pub fn read(build_state: &mut BuildCommandState) -> anyhow::Result = AHashMap::new(); let mut ast_rescript_file_locations = AHashSet::new(); + let mut source_path_module_names = AHashMap::new(); + let mut platform_cmt_module_names = AHashMap::new(); + for (module_name, module) in &build_state.modules { + let SourceType::SourceFile(source_file) = &module.source_type else { + continue; + }; + let package = build_state.packages.get(&module.package_name).unwrap(); + source_path_module_names.insert( + ( + module.package_name.clone(), + package.path.join(&source_file.implementation.path), + ), + module_name.clone(), + ); + if source_file + .implementation + .platform + .as_ref() + .is_none_or(|platform| platform.primary) + && let Some(interface) = &source_file.interface + { + source_path_module_names.insert( + (module.package_name.clone(), package.path.join(&interface.path)), + module_name.clone(), + ); + } + if let Some(platform) = &source_file.implementation.platform { + platform_cmt_module_names.insert( + ( + module.package_name.clone(), + format!("{}.{}", platform.logical_module_name, platform.name), + ), + module_name.clone(), + ); + } + } + let mut rescript_file_locations = build_state .modules .values() @@ -79,9 +116,11 @@ pub fn read(build_state: &mut BuildCommandState) -> anyhow::Result { - let module_name = helpers::file_path_to_module_name(path, package_namespace); - if let Some(res_file_path_buf) = get_res_path_from_ast(path) { + let module_name = source_path_module_names + .get(&(package_name.clone(), res_file_path_buf.clone())) + .cloned() + .unwrap_or_else(|| helpers::file_path_to_module_name(path, package_namespace)); let _ = ast_modules.insert( res_file_path_buf.clone(), AstModule { @@ -108,12 +147,16 @@ pub fn read(build_state: &mut BuildCommandState) -> anyhow::Result { - let module_name = helpers::file_path_to_module_name( + let physical_name = helpers::file_path_to_module_name( path, // we don't want to include a namespace here because the CMI file // already includes a namespace &packages::Namespace::NoNamespace, ); + let module_name = platform_cmt_module_names + .get(&(package_name.clone(), physical_name.clone())) + .cloned() + .unwrap_or(physical_name); cmt_modules.insert(module_name, last_modified.to_owned()); } _ => { diff --git a/rewatch/src/config.rs b/rewatch/src/config.rs index b01935e14c6..f4765b91179 100644 --- a/rewatch/src/config.rs +++ b/rewatch/src/config.rs @@ -555,6 +555,9 @@ pub struct Config { pub package_specs: Option>, pub warnings: Option, pub suffix: Option, + /// File-name suffixes recognized as platform implementations, for example + /// `Button.android.res` and `Button.ios.res`. + pub platforms: Option>, #[serde(alias = "bs-dependencies")] pub dependencies: Option>, #[serde(rename = "dev-dependencies", alias = "bs-dev-dependencies")] @@ -772,6 +775,26 @@ impl Config { } config.handle_deprecations()?; + if let Some(platforms) = &config.platforms { + if platforms.is_empty() { + return Err(anyhow!("'platforms' must contain at least one platform")); + } + let mut seen = std::collections::HashSet::new(); + for platform in platforms { + if platform.is_empty() + || !platform.chars().enumerate().all(|(index, ch)| { + ch.is_ascii_lowercase() || (index > 0 && (ch.is_ascii_digit() || ch == '_')) + }) + { + return Err(anyhow!( + "Invalid platform '{platform}'. Platform names must start with a lowercase ASCII letter and contain only lowercase letters, digits, and underscores" + )); + } + if !seen.insert(platform) { + return Err(anyhow!("Duplicate platform '{platform}'")); + } + } + } config.unknown_fields = unknown_fields; Ok(config) @@ -941,11 +964,14 @@ impl Config { /// Build the full set of `-bs-gentype-*` CLI flags for a bsc invocation. /// `source_dirs` are pre-expanded directories relative to the package root. + /// `suffix_override` replaces the configured JavaScript suffix when the + /// runtime import needs different resolution semantics. pub fn get_gentype_args( &self, source_dirs: &[PathBuf], bsb_project_root: Option<&Path>, dep_paths: &[(String, PathBuf)], + suffix_override: Option<&str>, ) -> Vec { let Some(gt) = &self.gentype_config else { return vec![]; @@ -978,9 +1004,9 @@ impl Config { args.push("-bs-gentype-generated-extension".to_string()); args.push(ext.clone()); } - if let Some(suffix) = &self.suffix { + if let Some(suffix) = suffix_override.or(self.suffix.as_deref()) { args.push("-bs-gentype-suffix".to_string()); - args.push(suffix.clone()); + args.push(suffix.to_string()); } let mut shims: Vec<(&String, &String)> = gt.shims.0.iter().collect(); shims.sort_by(|a, b| a.0.cmp(b.0)); @@ -1407,6 +1433,7 @@ pub mod tests { package_specs: None, warnings: None, suffix: None, + platforms: None, dependencies: Some(args.bs_deps.into_iter().map(Dependency::Shorthand).collect()), dev_dependencies: Some( args.build_dev_deps @@ -1504,6 +1531,28 @@ pub mod tests { assert_eq!(config.get_package_specs().len(), 2); } + #[test] + fn test_platforms_validation() { + let config = Config::new_from_json_string(r#"{"name":"platforms","platforms":["android","ios_17"]}"#) + .expect("valid platform names"); + assert_eq!( + config.platforms, + Some(vec!["android".to_string(), "ios_17".to_string()]) + ); + + for (platforms, expected) in [ + (r#"[]"#, "at least one platform"), + (r#"["ios","ios"]"#, "Duplicate platform 'ios'"), + (r#"["i-os"]"#, "Invalid platform 'i-os'"), + (r#"["17ios"]"#, "Invalid platform '17ios'"), + (r#"["IOS"]"#, "Invalid platform 'IOS'"), + ] { + let json = format!(r#"{{"name":"platforms","platforms":{platforms}}}"#); + let error = Config::new_from_json_string(&json).unwrap_err().to_string(); + assert!(error.contains(expected), "unexpected error: {error}"); + } + } + #[test] fn test_sources() { let json = r#" @@ -1592,7 +1641,7 @@ pub mod tests { assert_eq!(gt.module, Some(GenTypeModule::EsModule)); assert_eq!(gt.generated_file_extension.as_deref(), Some(".gen.tsx")); - let args = config.get_gentype_args(&[], None, &[]); + let args = config.get_gentype_args(&[], None, &[], None); assert!(args.contains(&"-bs-gentype".to_string())); assert!(args.contains(&"-bs-gentype-module".to_string())); assert!(args.contains(&"esmodule".to_string())); @@ -1602,6 +1651,13 @@ pub mod tests { assert!(args.contains(&".mjs".to_string())); assert!(args.contains(&"-bs-gentype-dep".to_string())); assert!(args.contains(&"@teamwalnut/app".to_string())); + + let platform_args = config.get_gentype_args(&[], None, &[], Some("")); + let suffix_idx = platform_args + .iter() + .position(|arg| arg == "-bs-gentype-suffix") + .unwrap(); + assert_eq!(platform_args[suffix_idx + 1], ""); } #[test] @@ -1656,7 +1712,7 @@ pub mod tests { } "#; let config = serde_json::from_str::(json).unwrap(); - let args = config.get_gentype_args(&[], None, &[]); + let args = config.get_gentype_args(&[], None, &[], None); let module_idx = args.iter().position(|s| s == "-bs-gentype-module").unwrap(); assert_eq!(args[module_idx + 1], "commonjs"); } @@ -1674,7 +1730,7 @@ pub mod tests { } "#; let config = serde_json::from_str::(json).unwrap(); - let args = config.get_gentype_args(&[], None, &[]); + let args = config.get_gentype_args(&[], None, &[], None); let module_idx = args.iter().position(|s| s == "-bs-gentype-module").unwrap(); assert_eq!(args[module_idx + 1], "esmodule"); } @@ -1688,7 +1744,7 @@ pub mod tests { } "#; let config = serde_json::from_str::(json).unwrap(); - assert!(config.get_gentype_args(&[], None, &[]).is_empty()); + assert!(config.get_gentype_args(&[], None, &[], None).is_empty()); } #[test] diff --git a/rewatch/src/helpers.rs b/rewatch/src/helpers.rs index 437fd05c1d5..c8812395fd7 100644 --- a/rewatch/src/helpers.rs +++ b/rewatch/src/helpers.rs @@ -336,6 +336,37 @@ pub fn file_path_to_module_name(path: &Path, namespace: &packages::Namespace) -> capitalize(&file_path_to_compiler_asset_basename(path, namespace)) } +/// Returns the platform suffix and the corresponding ordinary implementation +/// path. `src/Button.android.res` becomes (`android`, `src/Button.res`). +pub fn platform_implementation(path: &Path, platforms: &[String]) -> Option<(String, PathBuf)> { + if path.extension().and_then(|extension| extension.to_str()) != Some("res") { + return None; + } + let stem = path.file_stem()?.to_str()?; + let (logical_stem, platform) = stem.rsplit_once('.')?; + if logical_stem.is_empty() || !platforms.iter().any(|candidate| candidate == platform) { + return None; + } + let logical = path.with_file_name(format!("{logical_stem}.res")); + Some((platform.to_string(), logical)) +} + +/// Returns the platform suffix and common interface path for a platform-specific +/// interface. Platform module families deliberately use `Button.resi`, so this +/// recognizes `Button.android.resi` in order to report it as unsupported. +pub fn platform_interface(path: &Path, platforms: &[String]) -> Option<(String, PathBuf)> { + if path.extension().and_then(|extension| extension.to_str()) != Some("resi") { + return None; + } + let stem = path.file_stem()?.to_str()?; + let (logical_stem, platform) = stem.rsplit_once('.')?; + if logical_stem.is_empty() || !platforms.iter().any(|candidate| candidate == platform) { + return None; + } + let logical = path.with_file_name(format!("{logical_stem}.resi")); + Some((platform.to_string(), logical)) +} + pub fn contains_ascii_characters(str: &str) -> bool { for chr in str.chars() { if chr.is_ascii_alphanumeric() { @@ -550,3 +581,29 @@ pub fn is_local_package(workspace_path: &Path, canonical_package_path: &Path) -> .components() .any(|c| c.as_os_str() == "node_modules") } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn recognizes_configured_platform_sources() { + let platforms = vec!["android".to_string(), "ios".to_string()]; + assert_eq!( + platform_implementation(Path::new("src/Button.android.res"), &platforms), + Some(("android".to_string(), PathBuf::from("src/Button.res"))) + ); + assert_eq!( + platform_interface(Path::new("src/Button.ios.resi"), &platforms), + Some(("ios".to_string(), PathBuf::from("src/Button.resi"))) + ); + assert_eq!( + platform_implementation(Path::new("src/Button.web.res"), &platforms), + None + ); + assert_eq!( + platform_implementation(Path::new("src/Button.android.resi"), &platforms), + None + ); + } +} diff --git a/rewatch/src/watcher.rs b/rewatch/src/watcher.rs index 9bb738a77a8..be181d71aec 100644 --- a/rewatch/src/watcher.rs +++ b/rewatch/src/watcher.rs @@ -210,6 +210,50 @@ fn carry_forward_compile_warnings(previous: &BuildCommandState, next: &mut Build } } +fn mark_source_path_dirty(build_state: &mut BuildCommandState, source_path: &Path) -> bool { + let modified = source_path + .metadata() + .and_then(|metadata| metadata.modified()) + .ok(); + let mut matched = false; + + for (module_name, package_name) in build_state.module_name_package_pairs() { + let package_path = build_state + .build_state + .packages + .get(&package_name) + .expect("Package not found") + .path + .clone(); + let Some(module) = build_state.build_state.modules.get_mut(&module_name) else { + continue; + }; + let SourceType::SourceFile(source_file) = &mut module.source_type else { + continue; + }; + + if source_path == package_path.join(&source_file.implementation.path) { + if let Some(modified) = modified { + source_file.implementation.last_modified = modified; + } + source_file.implementation.parse_dirty = true; + return true; + } + + if let Some(interface) = &mut source_file.interface + && source_path == package_path.join(&interface.path) + { + if let Some(modified) = modified { + interface.last_modified = modified; + } + interface.parse_dirty = true; + matched = true; + } + } + + matched +} + fn should_clear_screen(clear_screen: bool, show_progress: bool, plain_output: bool) -> bool { clear_screen && show_progress && !plain_output } @@ -371,51 +415,7 @@ async fn async_watch( .canonicalize() .map(StrippedVerbatimPath::to_stripped_verbatim_path) { - // Collect package names first to avoid borrow checker issues - let module_package_pairs = build_state.module_name_package_pairs(); - - for (module_name, package_name) in module_package_pairs { - let package = build_state - .build_state - .packages - .get(&package_name) - .expect("Package not found"); - - if let Some(module) = build_state.build_state.modules.get_mut(&module_name) { - match module.source_type { - SourceType::SourceFile(ref mut source_file) => { - let canonicalized_implementation_file = - package.path.join(&source_file.implementation.path); - if canonicalized_path_buf == canonicalized_implementation_file { - if let Ok(modified) = - canonicalized_path_buf.metadata().and_then(|x| x.modified()) - { - source_file.implementation.last_modified = modified; - }; - source_file.implementation.parse_dirty = true; - break; - } - - // mark the interface file dirty - if let Some(ref mut interface) = source_file.interface { - let canonicalized_interface_file = - package.path.join(&interface.path); - if canonicalized_path_buf == canonicalized_interface_file { - if let Ok(modified) = canonicalized_path_buf - .metadata() - .and_then(|x| x.modified()) - { - interface.last_modified = modified; - } - interface.parse_dirty = true; - break; - } - } - } - SourceType::MlMap(_) => (), - } - } - } + mark_source_path_dirty(&mut build_state, &canonicalized_path_buf); needs_compile_type = CompileType::Incremental; } } @@ -768,6 +768,7 @@ mod tests { source_type: SourceType::SourceFile(SourceFile { implementation: Implementation { path: PathBuf::from(implementation_path), + platform: None, parse_state: ParseState::Success, compile_state: implementation_compile_state, last_modified: SystemTime::UNIX_EPOCH, @@ -870,4 +871,30 @@ mod tests { assert_eq!(interface.compile_warnings.as_deref(), Some("warning: interface")); assert_eq!(interface.compile_state, CompileState::Warning); } + + #[test] + fn shared_interface_edit_marks_every_platform_implementation_dirty() { + let mut state = test_build_state( + "Button", + test_module("src/Button.android.res", None, Some("src/Button.resi"), None), + ); + state.insert_module( + "Button$$platform$ios", + test_module("src/Button.ios.res", None, Some("src/Button.resi"), None), + ); + + assert!(mark_source_path_dirty( + &mut state, + Path::new("/tmp/rewatch-warning-carry-forward/src/Button.resi") + )); + + for module_name in ["Button", "Button$$platform$ios"] { + let SourceType::SourceFile(source_file) = &state.get_module(module_name).unwrap().source_type + else { + panic!("expected source file"); + }; + assert!(source_file.interface.as_ref().unwrap().parse_dirty); + assert!(!source_file.implementation.parse_dirty); + } + } } diff --git a/rewatch/tests/fixtures/react-native-platforms/.gitignore b/rewatch/tests/fixtures/react-native-platforms/.gitignore new file mode 100644 index 00000000000..cfd6f8d7fc6 --- /dev/null +++ b/rewatch/tests/fixtures/react-native-platforms/.gitignore @@ -0,0 +1,5 @@ +node_modules/ +.build/ +generated/ +bundles/ +lib/ diff --git a/rewatch/tests/fixtures/react-native-platforms/README.md b/rewatch/tests/fixtures/react-native-platforms/README.md new file mode 100644 index 00000000000..b6925ee8d3d --- /dev/null +++ b/rewatch/tests/fixtures/react-native-platforms/README.md @@ -0,0 +1,55 @@ +# React Native platform module fixture + +Permanent fixture for [`../../platforms`](../../platforms), promoted from the +original feasibility experiment. It has one `Button.resi`, Android/iOS +implementations with different abstract-type representations, and a shared +`App.res` that uses the logical `Button` module. + +The acceptance tests exercise Rewatch's `"platforms": ["android", "ios"]` +configuration directly. `build.mjs` invokes the real Rewatch binary and contains +no platform build logic itself. + +After building the checkout with `make lib`: + +```sh +cd rewatch/tests/fixtures/react-native-platforms +npm test +``` + +This command needs no npm installation. It runs the core acceptance tests in +temporary projects. To select a different Rewatch executable, set +`REWATCH_EXECUTABLE`; `RESCRIPT_BSC_EXE` and `RESCRIPT_RUNTIME` override the local +compiler and runtime. + +For the separate React Native/Metro integration check: + +```sh +npm ci +npm run bundle +``` + +That command first runs Rewatch, builds Android and iOS bundles, and checks their source maps for +the correct variants. The fixture uses CommonJS with out-of-source `.js` output +under `lib/js/src`; `index.cjs` registers `PlatformExperiment` from that output. +The bundler requires one shared `App.js` output. + +No simulator or native project scaffolding is needed: this tests the +compiler/build-system/bundler boundary. The core test stubs React and React Native +when executing generated modules. The bundle test uses the pinned real packages. +The shared interface and both platform implementations declare +`@react.component`, so JSX v4 derives their `props` type from `~title`. The +platform components render the native button through small local React and React +Native bindings, and the annotated `App` component renders the logical +`