Web build pipeline fixes for #148

ai-skills/skills/valdi-custom-view/skill.md

@@ -53,7 +53,7 @@ class MyComponent extends Component<MyViewModel> {
 }
 ```
 
-`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 `<custom-view>` 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 `<custom-view>` 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
 

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",
 ])

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;

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);
     }

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,

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: <module>/web/<file> 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/<flavor>/res directories; those are handled
+        # by the web/<flavor>/{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/<module_name>/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 <module_name>/src/...
         # Return it as src/<module_name>/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. <module>/src/*.d.ts) have no web/ segment, so _dest_native falls back to
+        # the raw short_path (../<external_repo>/...) 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):

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,

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),

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)")
         }
     }

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 `<custom-view>` 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 `<custom-view>` 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 `<custom-view>`
 
 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<string, ViewFactory> = {
 };
 ```
 
-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`: