Return a static slice from all_module_ids instead of allocating a Vec

[dhruv8sh]

Summary

• all_module_ids() built a fresh Vec<&'static str> on every call even though the answer never changes; it now returns a &'static [&'static str] backed by a build-script-generated static array, so the parse_single_module_filename lookup on the per-request deferred-module path no longer allocates.
• Chose the static-slice shape over an iterator: build.rs already generates TSJS_MODULES as a compile-time array, so extending that codegen to also emit ALL_MODULE_IDS keeps the same pattern and avoids the extra .collect() an iterator-returning version would force on publisher.rs:1970's concatenated_hash call.
• Fixed a clippy::redundant_static_lifetimes failure this introduced in the generated code, and dropped a redundant & at the concatenated_hash call site now that all_module_ids() already returns a reference.

Changes

File	Change
crates/trusted-server-js/build.rs	Generates a new pub(crate) const ALL_MODULE_IDS: [&str; N] array alongside TSJS_MODULES, written via writeln! instead of a per-iteration String allocation
crates/trusted-server-js/src/bundle.rs	all_module_ids() is now a const fn returning &ALL_MODULE_IDS (&'static [&'static str]) instead of collecting a Vec
crates/trusted-server-core/src/publisher.rs	parse_single_module_filename scans the static slice directly (.iter().copied().find(...)) instead of allocating a Vec first; removed a redundant & on the all_module_ids() call passed to concatenated_hash

Closes

Closes #435

Test plan

• cargo test-fastly && cargo test-axum
• cargo clippy-fastly && cargo clippy-axum
• cargo fmt --all -- --check
• JS tests: cd crates/trusted-server-js/lib && npx vitest run
• JS format: cd crates/trusted-server-js/lib && npm run format
• Docs format: cd docs && npm run format
• WASM build: cargo build --package trusted-server-adapter-fastly --release --target wasm32-wasip1
• Manual testing via fastly compute serve
• Other: cargo clippy -p trusted-server-js --all-targets -- -D warnings and cargo clippy -p trusted-server-core --all-targets -- -D warnings directly, plus cargo test -p trusted-server-js (existing bundle tests at bundle.rs:143,157 cover the new return type unchanged)

Checklist

• Changes follow AGENTS.md conventions
• No unwrap() in production code — use expect("should ...")
• Uses tracing macros (not println!)
• New code has tests
• No secrets or credentials committed

[ChristianPavilonis]

Summary

Reviewed a5541e29baca3e8fba44b964821e0c1d82dcf317 against 6cae7f5da8911c746cf873581885f90c3820dd96. The generated static module-ID slice preserves the membership and order of TSJS_MODULES, and the affected deferred-module routing and template-fingerprint paths pass their focused tests. No actionable issues found.

[prk-Jr]

Summary

Reviewed commit a5541e29baca3e8fba44b964821e0c1d82dcf317.

The generated static module-ID slice preserves the membership and core-first ordering of TSJS_MODULES. Workspace callers use the returned static references correctly in deferred-module lookup and template fingerprinting. No actionable findings.

The exact-head native bundle test suite passed (2 tests), and the build script generated all 13 JS modules. General adapter/build/lint gates rely on remote CI.

CI Status

• browser integration tests: PASS
• integration tests: PASS
• integration tests (Fastly EC lifecycle): PASS
• CodeQL: PASS
• cargo test (ts CLI, native): PASS
• cargo test (cross-adapter parity): PASS
• Analyze (javascript-typescript): PASS
• CLAUDE.md symlink guard: PASS
• cargo fmt: PASS (required)
• cargo test (axum native): PASS
• Analyze (actions): PASS
• format-typescript: PASS (required)
• cargo check (cloudflare native + wasm32-unknown-unknown): PASS
• cargo test: PASS (required)
• format-docs: PASS (required)
• vitest: PASS
• Analyze (rust): PASS
• prepare integration artifacts: PASS
• cargo check/build/test (spin native + wasm32-wasip1): PASS
• Analyze (javascript-typescript): PASS

[aram356]

Summary

Correct, focused change: all_module_ids() no longer allocates on the per-request parse_single_module_filename path, and both call-site edits are behaviour-preserving. One structural concern blocks: the codegen now emits two independent module-ID lists that can drift apart silently.

Verified independently on a5541e29b: cargo fmt --check, cargo clippy-fastly, cargo test-axum (26 passed), cargo test -p trusted-server-core … tsjs (49 passed), cargo test -p trusted-server-js (2 passed) — all green.

Neither inline comment carries a one-click suggestion: the blocking fix also deletes lines in two other diff hunks, and the test addition lands outside any hunk. Both comments carry the proposed code in full with an apply-manually note.

Blocking

🔧 wrench

• Codegen emits two independent module-ID lists that can silently drift — see inline at crates/trusted-server-js/build.rs:154-161

Non-blocking

🌱 seedling

• Add a regression test pinning the two generated lists together — see inline at crates/trusted-server-js/src/bundle.rs:19-20

CI Status

• browser integration tests: PASS
• integration tests: PASS
• integration tests (Fastly EC lifecycle): PASS
• CodeQL: PASS
• cargo test (ts CLI, native): PASS
• cargo test (cross-adapter parity): PASS
• Analyze (javascript-typescript): PASS
• CLAUDE.md symlink guard: PASS
• cargo fmt: PASS (required)
• cargo test (axum native): PASS
• Analyze (actions): PASS
• format-typescript: PASS (required)
• cargo check (cloudflare native + wasm32-unknown-unknown): PASS
• cargo test: PASS (required)
• format-docs: PASS (required)
• vitest: PASS
• Analyze (rust): PASS
• prepare integration artifacts: PASS
• cargo check/build/test (spin native + wasm32-wasip1): PASS

[aram356]

🔧 wrench — Two generated module-ID lists that can silently drift.

ALL_MODULE_IDS is accumulated in a separate loop variable (ids_block, line 144) from the one that emits TSJS_MODULES (line 147). That is two generated lists from one source, with nothing coupling them. Every runtime consumer splits across the two:

• module_bundle() / single_module_hash() / js_module_ids() read the TSJS_MODULES map
• parse_single_module_filename() (crates/trusted-server-core/src/publisher.rs:610) reads ALL_MODULE_IDS

I verified the drift is both real and invisible to the test suite. Patching build.rs to drop one module (testlight) from ids_block only, then rebuilding, produced a generated file with TSJS_MODULES: [_; 13] against ALL_MODULE_IDS: [_; 12] — and the entire trusted-server-js suite still passed (2/2 ok). No test asserts the two lists agree.

Failure scenario: a deferred module present in TSJS_MODULES but missing from ALL_MODULE_IDS still gets a <script defer src="/static/tsjs=tsjs-<id>.min.js"> tag emitted, because that path gates on module_bundle(). But parse_single_module_filename returns None, so handle_tsjs_dynamic answers not_found_response(). The integration silently stops loading in the browser, with green CI.

Proposed fix — derive the array from TSJS_MODULES in a const block so only one list exists. Delete the ids_block declaration (line 134) and its writeln! (line 144), then replace lines 154-161 with:

    // Derived from `TSJS_MODULES` in a const block so the two can never drift:
    // there is one generated list of IDs, not two.
    writeln!(
        codegen,
        "pub(crate) const ALL_MODULE_IDS: [&str; {0}] = {{\n    let mut ids = [\"\"; {0}];\n    let mut index = 0;\n    while index < {0} {{\n        ids[index] = TSJS_MODULES[index].id;\n        index += 1;\n    }}\n    ids\n}};",
        modules.len()
    )
    .expect("should write generated module IDs");

I applied this in a scratch worktree and verified it: it builds, generates an identical 13-entry list, cargo test -p trusted-server-js passes, and cargo clippy -p trusted-server-js --all-targets -- -D warnings is clean.

Apply manually — can't be auto-applied as a suggestion because the change also deletes lines 134 and 144, which sit in two other diff hunks.

For the record: #435 proposed a OnceLock + &'static [&'static str] shape, which would also have had a single source of truth. The PR body's justification for the static-array choice on .collect() grounds is fair — the const-derived form above keeps that benefit and closes the drift.

[dhruv8sh]

Applied — ALL_MODULE_IDS is now derived from TSJS_MODULES in a single const block, so there's only one generated list. Your proposed snippet had let mut ids = [...]; let mut index = 0;; I initially wrote it as let mut (index, ids) = (0, [...]); which doesn't compile (mut can't attach to a tuple pattern like that) — fixed to match your verified form. cargo test-fastly -p trusted-server-js, cargo test-axum, and cargo fmt --check all pass.

[aram356]

🌱 seedling — Pin the two generated lists together with a regression test.

Even with the const-derived fix in build.rs, a cheap test documents the invariant for whoever edits the codegen next:

#[test]
fn all_module_ids_matches_generated_module_list() {
    let from_modules: Vec<&str> = TSJS_MODULES.iter().map(|module| module.id).collect();
    assert_eq!(
        all_module_ids(),
        from_modules.as_slice(),
        "the generated ID list should match the generated module table"
    );
}

Verified passing against the derived version in a scratch worktree.

Apply manually — can't be auto-applied as a suggestion because it adds code to the #[cfg(test)] module at the bottom of this file, outside any diff hunk.

[dhruv8sh]

Added — all_module_ids_matches_generated_module_list in bundle.rs, matching what you proposed. Passes: bundle::tests::all_module_ids_matches_generated_module_list ... ok.