From df12566d84845274cb41791154abb78496bec50a Mon Sep 17 00:00:00 2001 From: Beau Collins Date: Thu, 27 Aug 2026 09:33:35 -0700 Subject: [PATCH] Web build pipeline fixes Fixes found while integrating the web build into a downstream consumer: - valdi_compiled.bzl: declare web output flavor from the compiler's web_output_target (release only when android+ios targets are release and the build isn't forced to debug) instead of output_flavor alone, so debug-target modules (e.g. jasmine) no longer mis-declare web/release outputs the compiler never writes. - CollapseWebPaths.cpp: recursively collect module res directories and break out of the per-dir image scan so transitively-staged res dirs still register (previously blank icons at runtime). - valdi_collapse_web_paths.bzl: skip non-web source files in the native pass so paths that would escape the -o output tree no longer fail under a strict sandbox; keep real module names for compiler-generated web//res. - web_path_browserify_shim.js.tpl: preserve leading ".." segments when normalizing relative paths. - web_webpack.config.js.tpl + web_bytes_loader.js.tpl + wiring: replace the non-existent 'asset/bytes' module type with a custom loader that emits .bin/.protodecl resources as Uint8Array. - HotReloadLifecycleReporter.swift: serialize lifecycle writes with an NSLock so concurrent lines from different queues are not interleaved. - docs + skill: clarify that a custom-view binds via exactly one mechanism (viewFactory or a platform class), never both. Based on the web build branch. --- ai-skills/skills/valdi-custom-view/skill.md | 2 +- bzl/valdi/app_templates/BUILD.bazel | 1 + .../app_templates/web_bytes_loader.js.tpl | 13 ++++ .../web_path_browserify_shim.js.tpl | 6 +- .../app_templates/web_webpack.config.js.tpl | 4 +- bzl/valdi/valdi_collapse_web_paths.bzl | 47 +++++------- bzl/valdi/valdi_compiled.bzl | 52 +++++++------ bzl/valdi/valdi_web_application.bzl | 9 +++ .../Reloader/HotReloadLifecycleReporter.swift | 10 ++- docs/docs/native-customviews.md | 11 ++- .../compiler_toolbox/CollapseWebPaths.cpp | 73 +++++++++++++++---- .../toolbox/test/CollapseWebPaths_tests.cpp | 7 +- 12 files changed, 156 insertions(+), 79 deletions(-) create mode 100644 bzl/valdi/app_templates/web_bytes_loader.js.tpl diff --git a/ai-skills/skills/valdi-custom-view/skill.md b/ai-skills/skills/valdi-custom-view/skill.md index f6821c3a0..60c791119 100644 --- a/ai-skills/skills/valdi-custom-view/skill.md +++ b/ai-skills/skills/valdi-custom-view/skill.md @@ -53,7 +53,7 @@ class MyComponent extends Component { } ``` -`viewFactory` and `*Class` attributes are mutually exclusive — use one or the other. `viewFactory` takes precedence when both are provided. +`viewFactory` and `*Class` attributes are mutually exclusive — use one or the other. Specifying both on the same `` is a compile error (the JSXProcessor rejects it). If a view needs a `viewFactory` on one platform and a registered class on another, branch into two sibling `` elements, each with a single mechanism. Prefer gating on whether a `viewFactory` is provided (`if (viewFactory) { … } else { …class… }`) over `Device.isWeb()`: the factory is often supplied by native too, so an `isWeb` branch that routes native to classes can break it. Only keep a platform class you've confirmed actually exists. ## macOS Attribute Binding diff --git a/bzl/valdi/app_templates/BUILD.bazel b/bzl/valdi/app_templates/BUILD.bazel index 56ec48b0a..ab3e6e00d 100644 --- a/bzl/valdi/app_templates/BUILD.bazel +++ b/bzl/valdi/app_templates/BUILD.bazel @@ -11,5 +11,6 @@ exports_files([ "web_index.html.tpl", "web_index.js.tpl", "web_path_browserify_shim.js.tpl", + "web_bytes_loader.js.tpl", "web_webpack.config.js.tpl", ]) diff --git a/bzl/valdi/app_templates/web_bytes_loader.js.tpl b/bzl/valdi/app_templates/web_bytes_loader.js.tpl new file mode 100644 index 000000000..724f0ab21 --- /dev/null +++ b/bzl/valdi/app_templates/web_bytes_loader.js.tpl @@ -0,0 +1,13 @@ +// Loads .bin/.protodecl resources as raw bytes (Uint8Array). The web runtime's +// resourceModuleToByteArray only accepts Uint8Array/ArrayBuffer, and webpack has no +// built-in byte-array module type, so emit a module that decodes base64 to bytes. +module.exports = function bytesLoader(content) { + const base64 = content.toString('base64'); + return ( + `const s = atob(${JSON.stringify(base64)});` + + 'const bytes = new Uint8Array(s.length);' + + 'for (let i = 0; i < s.length; i++) { bytes[i] = s.charCodeAt(i); }' + + 'module.exports = bytes;' + ); +}; +module.exports.raw = true; diff --git a/bzl/valdi/app_templates/web_path_browserify_shim.js.tpl b/bzl/valdi/app_templates/web_path_browserify_shim.js.tpl index 87f9e3667..02020f4b0 100644 --- a/bzl/valdi/app_templates/web_path_browserify_shim.js.tpl +++ b/bzl/valdi/app_templates/web_path_browserify_shim.js.tpl @@ -10,7 +10,11 @@ function normalize(value) { continue; } if (part === '..') { - out.pop(); + if (out.length === 0 || out[out.length - 1] === '..') { + if (!absolute) out.push('..'); + } else { + out.pop(); + } } else { out.push(part); } diff --git a/bzl/valdi/app_templates/web_webpack.config.js.tpl b/bzl/valdi/app_templates/web_webpack.config.js.tpl index f4618626c..a905a8340 100644 --- a/bzl/valdi/app_templates/web_webpack.config.js.tpl +++ b/bzl/valdi/app_templates/web_webpack.config.js.tpl @@ -68,8 +68,10 @@ module.exports = { type: 'javascript/auto', }, { + // Webpack has no built-in byte-array module type; use the custom loader. test: /\.(bin|protodecl)$/i, - type: 'asset/bytes', + use: [{ loader: path.resolve(__dirname, 'src/bytes-loader.js') }], + type: 'javascript/auto', }, { test: /\.(png|jpe?g|svg|webp)$/i, diff --git a/bzl/valdi/valdi_collapse_web_paths.bzl b/bzl/valdi/valdi_collapse_web_paths.bzl index 1940ff7a4..6395d87b4 100644 --- a/bzl/valdi/valdi_collapse_web_paths.bzl +++ b/bzl/valdi/valdi_collapse_web_paths.bzl @@ -1,22 +1,6 @@ load(":valdi_compiled.bzl", "ValdiModuleInfo") load(":valdi_web_workers.bzl", "validate_web_workers") -def _repository_relative_short_path(file): - """Returns a File.short_path relative to the root of its owning repository.""" - rel = file.short_path - if not rel.startswith("../"): - return rel - - repo_name = file.owner.repo_name - if not repo_name: - fail("External file has no owning repository: {}".format(rel)) - - external_prefix = "../{}/".format(repo_name) - if not rel.startswith(external_prefix): - fail("External file path {} does not match owning repository {}".format(rel, repo_name)) - - return rel[len(external_prefix):] - def _dest_native(rel): """Canonical path for a file in the native/ tree: /web/ or module path. @@ -107,7 +91,9 @@ def _dest(rel): parts = rel.split("/") for i in range(1, len(parts)): - if parts[i] == "res": + # Skip the compiler-generated web//res directories; those are handled + # by the web//{assets,res} logic below so the real module name is kept. + if parts[i] == "res" and parts[i - 1] not in ["debug", "release"]: module_name = parts[i - 1] tail = "/".join(parts[i:]) return "src/{}/{}".format(module_name, tail) @@ -119,15 +105,12 @@ def _dest(rel): # Try to find and strip the valdi marker from the path rel2 = rel - is_valdi_source_path = False if valdi_marker in rel: idx = rel.find(valdi_marker) rel2 = rel[idx + len(valdi_marker):] - is_valdi_source_path = True elif rel.startswith("src/valdi_modules/src/valdi/"): # Handle direct paths (non-external) rel2 = rel[len("src/valdi_modules/src/valdi/"):] - is_valdi_source_path = True parts = rel2.split("/") @@ -148,9 +131,9 @@ def _dest(rel): tail = "/".join(parts[i + 4:]) return "src/{}".format(tail) - # Handle source .d.ts files rooted under Valdi's source tree. + # Handle source .d.ts files from any path containing /src/valdi_modules/src/valdi/ # These should go into src//src/... - if rel.endswith(".d.ts") and is_valdi_source_path: + if rel.endswith(".d.ts") and valdi_marker in rel: # rel2 already has the marker stripped, so it's /src/... # Return it as src//src/... return "src/{}".format(rel2) @@ -170,12 +153,11 @@ def _impl(ctx): seen_dest = {} lines = [] for f in ctx.files.srcs: - rel = _repository_relative_short_path(f) - if exclude_jsx and "valdi_tsx/src/JSX.d.ts" in rel: + if exclude_jsx and "valdi_tsx/src/JSX.d.ts" in f.short_path: continue - if _should_exclude_from_package(rel): + if _should_exclude_from_package(f.short_path): continue - d = _dest(rel) + d = _dest(f.short_path) if d not in seen_dest: seen_dest[d] = True lines.append("{}\t{}".format(f.path, d)) @@ -296,8 +278,14 @@ def _impl_native(ctx): manifest = ctx.actions.declare_file(ctx.label.name + ".manifest") lines = [] for f in ctx.files.srcs: - rel = _repository_relative_short_path(f) - lines.append("{}\t{}".format(f.path, _dest_native(rel))) + # Only files under a module's web/ dir belong in the native tree. Source files + # (e.g. /src/*.d.ts) have no web/ segment, so _dest_native falls back to + # the raw short_path (..//...) which would escape the -o directory + # and fail under a strict sandbox. Those declarations are placed by the main + # collapse_web_paths pass instead. + if "web" not in f.short_path.split("/"): + continue + lines.append("{}\t{}".format(f.path, _dest_native(f.short_path))) ctx.actions.write(manifest, "\n".join(lines)) compiler_toolbox = ctx.executable._compiler_toolbox @@ -366,8 +354,7 @@ def _generate_register_native_modules_impl(ctx): seen_dest = {} n = 0 for f in ctx.files.srcs: - rel = _repository_relative_short_path(f) - dest = _dest_native(rel) + dest = _dest_native(f.short_path) if not dest.endswith(".js"): continue if not _should_register_native_module(dest): diff --git a/bzl/valdi/valdi_compiled.bzl b/bzl/valdi/valdi_compiled.bzl index 7ea95022d..10c668a56 100644 --- a/bzl/valdi/valdi_compiled.bzl +++ b/bzl/valdi/valdi_compiled.bzl @@ -893,15 +893,24 @@ def _get_web_output_target(attr, force_debug): def _supports_web_release(ctx, code_coverage): return _get_web_output_target(ctx.attr, code_coverage or ctx.attr.output_flavor[BuildSettingInfo].value == "debug") == "release" +def _web_output_flavor(ctx, code_coverage): + # Must equal the compiler's web_output_target (see _get_web_output_target, passed to + # the compiler as output_target): a module emits/consumes web/release only when both + # android and ios output_target are release and the build isn't forced to debug; + # otherwise web/debug. Keying off output_flavor alone mis-declares web/release for + # debug-output_target modules (e.g. jasmine), whose compiler only writes web/debug. + return _get_web_output_target(ctx.attr, code_coverage or ctx.attr.output_flavor[BuildSettingInfo].value == "debug") + def _get_files_output_paths(ctx, module_name, module_directory, localization_mode, enable_web, code_coverage, enable_android = True, enable_ios = True, emit_debug = True, emit_release = True): filtered_srcs = _get_compiled_srcs(ctx) outputs = _get_srcs_dts_paths(filtered_srcs, module_name, module_directory) - web_release_enabled = enable_web and _supports_web_release(ctx, code_coverage) - if web_release_enabled: - outputs += _get_srcs_js_paths(filtered_srcs + ctx.files.protodecl_srcs, module_name, module_directory, bool(ctx.files.res), bool(ctx.file.ids_yaml), bool(ctx.attr.strings_dir)) - outputs += _get_web_native_module_package_file_paths(ctx.attr.web_register_native_module_id_overrides) - outputs += _get_srcs_vue_paths(ctx.files.legacy_vue_srcs, module_name, module_directory) + web_flavor = _web_output_flavor(ctx, code_coverage) + web_res_index = 0 if web_flavor == "debug" else 1 + if enable_web: + outputs += _get_srcs_js_paths(filtered_srcs + ctx.files.protodecl_srcs, module_name, module_directory, bool(ctx.files.res), bool(ctx.file.ids_yaml), bool(ctx.attr.strings_dir), web_flavor) + outputs += _get_web_native_module_package_file_paths(ctx.attr.web_register_native_module_id_overrides, web_flavor) + outputs += _get_srcs_vue_paths(ctx.files.legacy_vue_srcs, module_name, module_directory, web_flavor) outputs += get_sql_js_paths(ctx.attr.sql_db_names, ctx.files.sql_srcs, module_name, module_directory) outputs += get_legacy_vue_srcs_dts_paths(ctx.files.legacy_vue_srcs, module_name, module_directory) @@ -945,8 +954,8 @@ def _get_files_output_paths(ctx, module_name, module_directory, localization_mod strings_json_srcs = ctx.files.strings_json_srcs outputs += get_strings_dts_path(TYPESCRIPT_GENERATED_TS_DIR, module_name, strings_json_srcs) - if web_release_enabled: - outputs += _get_web_string_resource_paths(module_name, strings_json_srcs, ctx.attr.strings_dir)[1] + if enable_web: + outputs += _get_web_string_resource_paths(module_name, strings_json_srcs, ctx.attr.strings_dir)[web_res_index] if localization_mode == "external": # Android strings-xx.xml @@ -967,9 +976,9 @@ def _get_files_output_paths(ctx, module_name, module_directory, localization_mod outputs = _append_debug_and_maybe_release(outputs, android_output_target, _get_android_image_resources_paths(module_name, basenames), emit_debug, emit_release) if enable_ios: outputs = _append_debug_and_maybe_release(outputs, ios_output_target, _get_ios_image_resources_paths(module_name, basenames), emit_debug, emit_release) - if web_release_enabled: + if enable_web: renamed_resources = _extract_renamed_resources(ctx.files.res) - outputs += _get_web_resource_paths(module_name, renamed_resources)[1] + outputs += _get_web_resource_paths(module_name, renamed_resources)[web_res_index] outputs += _get_web_generated_resource_paths(module_name, outputs) outputs.append(_get_dumped_compilation_metadata(module_name)) @@ -1140,9 +1149,8 @@ def _get_srcs_dts_paths(srcs, module_name, module_directory): if f.extension in ["tsx", "ts"] and not f.basename.endswith(".d.ts") ] -def _get_srcs_js_paths(srcs, module_name, module_directory, has_resources, has_ids, has_strings): +def _get_srcs_js_paths(srcs, module_name, module_directory, has_resources, has_ids, has_strings, output_target): out = [] - output_target = "release" for f in srcs: if _is_test_file(f): @@ -1159,8 +1167,8 @@ def _get_srcs_js_paths(srcs, module_name, module_directory, has_resources, has_i return out -def _get_web_native_module_package_file_paths(web_register_native_module_id_overrides): - output_dir = base_relative_dir("web", "release", "assets") +def _get_web_native_module_package_file_paths(web_register_native_module_id_overrides, output_target): + output_dir = base_relative_dir("web", output_target, "assets") out = [] for implementation_path in web_register_native_module_id_overrides: @@ -1172,9 +1180,8 @@ def _get_web_native_module_package_file_paths(web_register_native_module_id_over return out -def _get_srcs_vue_paths(srcs, module_name, module_directory): +def _get_srcs_vue_paths(srcs, module_name, module_directory, output_target): out = [] - output_target = "release" for f in srcs: if _is_test_file(f): @@ -1911,11 +1918,12 @@ def _create_valdi_module_info(ctx, module_name, module_yaml, module_definition, base_path = paths.join(ctx.label.workspace_root, ctx.label.package) single_file_codegen = ctx.attr.single_file_codegen - web_release_enabled = _supports_web_release(ctx, code_coverage) - web_input_dts_files = _extract_web_dts_files(in_declarations) if web_release_enabled else [] - web_output_dts_files = _extract_web_dts_files(out_declarations) if web_release_enabled else [] - web_resource_files = _extract_web_resources("release", outputs) if web_release_enabled else [] - if web_release_enabled and ctx.attr.inline_assets: + enable_web = ctx.var.get("enable_web") + web_flavor = _web_output_flavor(ctx, code_coverage) + web_input_dts_files = _extract_web_dts_files(in_declarations) if enable_web else [] + web_output_dts_files = _extract_web_dts_files(out_declarations) if enable_web else [] + web_resource_files = _extract_web_resources(web_flavor, outputs) if enable_web else [] + if enable_web and ctx.attr.inline_assets: web_resource_files += ctx.files.res return ValdiModuleInfo( @@ -1979,11 +1987,11 @@ def _create_valdi_module_info(ctx, module_name, module_yaml, module_definition, # web outputs protodecl_srcs = ctx.files.protodecl_srcs, - web_sources = _extract_js_files(module_name, "release", outputs) if web_release_enabled else [], + web_sources = _extract_js_files(module_name, web_flavor, outputs) if enable_web else [], web_resource_files = web_resource_files, web_no_inline_images = ctx.attr.web_no_inline_images, web_module_file_entries = _collect_web_module_file_entries(ctx, module_name), - web_strings = _extract_web_strings("release", outputs) if web_release_enabled else [], + web_strings = _extract_web_strings(web_flavor, outputs) if enable_web else [], web_deps = _extract_npm_package_files(ctx.attr.web_deps), web_input_dts_files = web_input_dts_files, web_output_dts_files = web_output_dts_files, diff --git a/bzl/valdi/valdi_web_application.bzl b/bzl/valdi/valdi_web_application.bzl index b1723ed49..8d37fe9c8 100644 --- a/bzl/valdi/valdi_web_application.bzl +++ b/bzl/valdi/valdi_web_application.bzl @@ -109,6 +109,7 @@ def valdi_web_application( html_target = "{}_html".format(name) entry_target = "{}_entry".format(name) path_shim_target = "{}_path_browserify_shim".format(name) + bytes_loader_target = "{}_bytes_loader".format(name) api_version_target = "{}_api_version".format(name) webpack_config_target = "{}_webpack_config".format(name) webpack_target = "{}_webpack".format(name) @@ -141,6 +142,13 @@ def valdi_web_application( substitutions = {}, ) + expand_template( + name = bytes_loader_target, + src = "@valdi//bzl/valdi/app_templates:web_bytes_loader.js.tpl", + output = "{}/src/bytes-loader.js".format(build_dir), + substitutions = {}, + ) + web_api_version_json( name = api_version_target, output = "{}/src/valdi_api_version.json".format(build_dir), @@ -161,6 +169,7 @@ def valdi_web_application( srcs = [ ":{}".format(entry_target), ":{}".format(path_shim_target), + ":{}".format(bytes_loader_target), ":{}".format(api_version_target), ":{}".format(webpack_config_target), ":{}".format(web_package_name), diff --git a/compiler/compiler/Compiler/Sources/Reloader/HotReloadLifecycleReporter.swift b/compiler/compiler/Compiler/Sources/Reloader/HotReloadLifecycleReporter.swift index f24a6a3b0..19d2c2efe 100644 --- a/compiler/compiler/Compiler/Sources/Reloader/HotReloadLifecycleReporter.swift +++ b/compiler/compiler/Compiler/Sources/Reloader/HotReloadLifecycleReporter.swift @@ -25,6 +25,9 @@ final class HotReloadLifecycleReporter { private let port: Int? private let output: Output private let errorOutput: Output + // Emitted from multiple queues (DaemonService connection queue, AutoRecompiler main + // queue); serialize writes so concurrent lines are not interleaved. + private let writeLock = NSLock() init(target: String, port: Int?, @@ -102,8 +105,13 @@ final class HotReloadLifecycleReporter { do { let encoder = JSONEncoder() encoder.outputFormatting = [.sortedKeys, .withoutEscapingSlashes] - output(String(decoding: try encoder.encode(payload), as: UTF8.self)) + let line = String(decoding: try encoder.encode(payload), as: UTF8.self) + writeLock.lock() + defer { writeLock.unlock() } + output(line) } catch { + writeLock.lock() + defer { writeLock.unlock() } errorOutput("Failed to encode Valdi hot reload lifecycle event '\(event)': \(error)") } } diff --git a/docs/docs/native-customviews.md b/docs/docs/native-customviews.md index 1a501480b..98a466bf8 100644 --- a/docs/docs/native-customviews.md +++ b/docs/docs/native-customviews.md @@ -6,6 +6,9 @@ Valdi supports injecting native views inside of an existing valdi feature throug This type of integration is useful when a very complex view (such as a system view) is already implemented natively and we want to re-use platform-specific code instead of writing a new cross-platform component. +> [!IMPORTANT] +> A `` binds its native view **one** of two ways: a `viewFactory` (a `ViewFactory` supplied at runtime, shown below) **or** platform class names (`iosClass`/`androidClass`/`macosClass`/`webClass`, covered further down). The two are mutually exclusive — specifying both on the same element is a compile error. If a view needs a `viewFactory` on one platform and a registered class on another, branch into two sibling `` elements, each with a single mechanism. Prefer gating on whether a `viewFactory` is provided (`if (viewFactory) { … } else { …class… }`) over `Device.isWeb()`, since the factory is often supplied by native too. Keep a platform class only when it actually exists. + ## Using a `` Here we find a simple example on how to inject custom views inside of a Valdi rendered feature. @@ -347,10 +350,9 @@ Web custom views use a factory registration pattern. Register a factory function interface AttributeHandler { changeAttribute(name: string, value: unknown): void; - destroy?(): void; } -type ViewFactory = (container: HTMLElement) => AttributeHandler | void; +type ViewFactory = (container: HTMLElement) => AttributeHandler; function createSliderFactory(): ViewFactory { return (container: HTMLElement): AttributeHandler => { @@ -364,9 +366,6 @@ function createSliderFactory(): ViewFactory { slider.value = String(value * 100); } }, - destroy(): void { - slider.remove(); - }, }; }; } @@ -377,7 +376,7 @@ export const webPolyglotViews: Record = { }; ``` -The factory function receives a container DOM element and can return an attribute handler. The handler receives updates through `changeAttribute(name, value)`. When the handler defines `destroy()`, Valdi calls it exactly once when the custom view is removed. Use `destroy()` to unmount framework roots, remove listeners, and release other resources owned by the custom view. +The factory function receives a container DOM element and returns an object with a `changeAttribute(name, value)` method to receive attribute updates from the Valdi renderer. To register web factories, create a `ts_project` (never a `filegroup`) and add it as `web_deps` in your `BUILD.bazel`. The `ts_project` requires `transpiler = "tsc"` and a dedicated `web/tsconfig.json`: diff --git a/valdi/compiler/toolbox/src/valdi/compiler_toolbox/CollapseWebPaths.cpp b/valdi/compiler/toolbox/src/valdi/compiler_toolbox/CollapseWebPaths.cpp index 2ce5f10ad..d8b4c2fcb 100644 --- a/valdi/compiler/toolbox/src/valdi/compiler_toolbox/CollapseWebPaths.cpp +++ b/valdi/compiler/toolbox/src/valdi/compiler_toolbox/CollapseWebPaths.cpp @@ -142,6 +142,8 @@ static void rewriteDeclarationImports(std::string& content, const std::string& p rewriteDeclarationImportsForPrefix(content, "from \"", '"', packageName); rewriteDeclarationImportsForPrefix(content, "import '", '\'', packageName); rewriteDeclarationImportsForPrefix(content, "import \"", '"', packageName); + rewriteDeclarationImportsForPrefix(content, "import('", '\'', packageName); + rewriteDeclarationImportsForPrefix(content, "import(\"", '"', packageName); } static Result copySourceDeclarations(const Path& outputDirectory, const Path& manifestPath) { @@ -271,26 +273,67 @@ static std::string camelCaseImageStem(std::string_view stem) { return result; } -static Result generateImageRegistry(const Path& sourceDirectory, const FlatSet& noInlineModules) { - std::vector moduleDirectories; - for (const auto& child : DiskUtils::listDirectory(sourceDirectory)) { +struct ModuleResDirectory { + std::string moduleName; + std::string relativePrefix; + Path directory; +}; + +// Recursively collect every module `res` directory that holds images. Direct +// dependencies land at `src//res`, but transitive dependencies are staged +// under `src//res//res`; scanning only the immediate children of +// `src/` misses the transitive ones, so their images are bundled yet never +// registered (blank icons at runtime). +static void collectModuleResDirectories(const Path& directory, + const std::string& relativePrefix, + std::string_view parentName, + std::vector& out) { + auto name = std::string(directory.getLastComponent()); + auto children = DiskUtils::listDirectory(directory); + if (name == "res") { + for (const auto& child : children) { + if (DiskUtils::isFile(child) && isImageExtension(child.getFileExtension())) { + // A res directory containing images belongs to its parent module. Stop the image + // scan but keep recursing subdirectories: transitive deps staged as res//res + // live below this dir and must still be collected. + out.push_back({std::string(parentName), relativePrefix, directory}); + break; + } + } + } + for (const auto& child : children) { if (DiskUtils::isDirectory(child)) { - moduleDirectories.push_back(child); + auto childName = std::string(child.getLastComponent()); + auto childPrefix = relativePrefix.empty() ? childName : relativePrefix + "/" + childName; + collectModuleResDirectories(child, childPrefix, name, out); } } - std::sort(moduleDirectories.begin(), moduleDirectories.end(), [](const auto& lhs, const auto& rhs) { - return lhs.getLastComponent() < rhs.getLastComponent(); +} + +static Result generateImageRegistry(const Path& sourceDirectory, const FlatSet& noInlineModules) { + std::vector resDirectories; + collectModuleResDirectories(sourceDirectory, "", sourceDirectory.getLastComponent(), resDirectories); + + // Shallower paths first so a top-level module res wins over a staged copy of the + // same module (identical images; the top-level require path is canonical). + std::sort(resDirectories.begin(), resDirectories.end(), [](const auto& lhs, const auto& rhs) { + auto lhsDepth = std::count(lhs.relativePrefix.begin(), lhs.relativePrefix.end(), '/'); + auto rhsDepth = std::count(rhs.relativePrefix.begin(), rhs.relativePrefix.end(), '/'); + if (lhsDepth != rhsDepth) { + return lhsDepth < rhsDepth; + } + return lhs.relativePrefix < rhs.relativePrefix; }); std::string content = "var __r = (globalThis.__valdiImageRegistry = globalThis.__valdiImageRegistry || {});\n"; - for (const auto& moduleDirectory : moduleDirectories) { - auto resourcesDirectory = moduleDirectory.appending("res"); - if (!DiskUtils::isDirectory(resourcesDirectory)) { + FlatSet seenModules; + for (const auto& resDirectory : resDirectories) { + if (seenModules.find(resDirectory.moduleName) != seenModules.end()) { continue; } std::vector images; - for (const auto& resource : DiskUtils::listDirectory(resourcesDirectory)) { + for (const auto& resource : DiskUtils::listDirectory(resDirectory.directory)) { if (DiskUtils::isFile(resource) && isImageExtension(resource.getFileExtension())) { images.push_back(resource); } @@ -299,8 +342,7 @@ static Result generateImageRegistry(const Path& sourceDirectory, const Fla return lhs.getLastComponent() < rhs.getLastComponent(); }); - auto moduleName = std::string(moduleDirectory.getLastComponent()); - auto noInlineImages = noInlineModules.find(moduleName) != noInlineModules.end(); + auto noInlineImages = noInlineModules.find(resDirectory.moduleName) != noInlineModules.end(); std::string entries; for (const auto& image : images) { auto filename = std::string(image.getLastComponent()); @@ -316,11 +358,12 @@ static Result generateImageRegistry(const Path& sourceDirectory, const Fla if (noInlineImages) { resourceQuery.append(resourceQuery.empty() ? "?no-inline" : "&no-inline"); } - entries.append( - fmt::format(" '{}': require('./{}/res/{}{}'),\n", key, moduleName, filename, resourceQuery)); + entries.append(fmt::format( + " '{}': require('./{}/{}{}'),\n", key, resDirectory.relativePrefix, filename, resourceQuery)); } if (!entries.empty()) { - content.append(fmt::format("__r['{}/res'] = {{\n{}}};\n", moduleName, entries)); + seenModules.insert(resDirectory.moduleName); + content.append(fmt::format("__r['{}/res'] = {{\n{}}};\n", resDirectory.moduleName, entries)); } } return DiskUtils::store(sourceDirectory.appending("_image_registry.js"), content); diff --git a/valdi/compiler/toolbox/test/CollapseWebPaths_tests.cpp b/valdi/compiler/toolbox/test/CollapseWebPaths_tests.cpp index 207dd0d4d..d0b3d0baa 100644 --- a/valdi/compiler/toolbox/test/CollapseWebPaths_tests.cpp +++ b/valdi/compiler/toolbox/test/CollapseWebPaths_tests.cpp @@ -73,7 +73,9 @@ TEST(CollapseWebPaths, buildsTheCompleteWebPackage) { "module.exports = require('core/src/Core');\n"); auto worker = directory.write("inputs/app/src/Worker.js", "workerService(module);\n"); auto strings = directory.write("inputs/app/src/Strings.js", "\"use strict\";\n"); - auto declaration = directory.write("inputs/app/src/Types.d.ts", "import { Core } from 'core/src/Core';\n"); + auto declaration = directory.write("inputs/app/src/Types.d.ts", + "import { Core } from 'core/src/Core';\n" + "export type Lazy = typeof import('core/src/Core');\n"); auto locale = directory.write("inputs/app/strings/en.json", "{}\n"); auto image = directory.write("inputs/app/res/music_icon.svg", "\n"); auto config = directory.write("inputs/app/res/config.json", "{}\n"); @@ -115,7 +117,8 @@ TEST(CollapseWebPaths, buildsTheCompleteWebPackage) { "NavigationPage)(module);\n" "module.exports = require('../../core/src/Core.js');\n"); EXPECT_EQ(directory.read("output/src/app/src/Types.d.ts"), - "import { Core } from '@scope/package/src/core/src/Core';\n"); + "import { Core } from '@scope/package/src/core/src/Core';\n" + "export type Lazy = typeof import('@scope/package/src/core/src/Core');\n"); EXPECT_EQ(directory.read("output/src/_navigation_registry.js"), "var __r = (globalThis.__valdiNavigationPages = globalThis.__valdiNavigationPages || {});\n" "__r['app/src/Main'] = function() { return require('./app/src/Main'); };\n");