Skip to content

Commit 3429fc0

Browse files
janechuCopilot
andcommitted
Address fast-build PR feedback
Preserve static attribute raw values, keep omitted-state behavior explicit, and use entry-template WASM rendering in the CLI. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent bd9b16a commit 3429fc0

10 files changed

Lines changed: 229 additions & 47 deletions

File tree

change/@microsoft-fast-build-9d7b843d-87a6-4f82-9db2-039234d8ad6e.json

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
{
2-
"type": "minor",
3-
"comment": "feat: allow fast-build rendering without state and omit missing bound attributes",
2+
"type": "major",
3+
"comment": "breaking: allow fast-build rendering without state; omitted CLI state no longer probes state.json",
44
"packageName": "@microsoft/fast-build",
55
"email": "7559015+janechu@users.noreply.github.com",
66
"dependentChangeType": "none"

crates/microsoft-fast-build/DESIGN.md

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,7 @@ render_template(template, state_str)
5454
| `json.rs` | Hand-rolled JSON parser producing `JsonValue` |
5555
| `locator.rs` | `Locator` struct — maps element names to template strings; glob scanner; `<f-template>` parser |
5656
| `error.rs` | `RenderError` enum with `Display` impl and helpers |
57-
| `wasm.rs` | WASM bindings (`#[cfg(target_arch = "wasm32")]`) — exposes `render`, `render_with_templates`, and `parse_f_templates` to JavaScript |
57+
| `wasm.rs` | WASM bindings (`#[cfg(target_arch = "wasm32")]`) — exposes `render`, `render_with_templates`, `render_entry_with_templates`, and `parse_f_templates` to JavaScript |
5858

5959
---
6060

@@ -302,9 +302,10 @@ All render functions accept `config: Option<&RenderConfig>` as their last parame
302302
- `render_entry_template_with_locator(template, state_str, locator, config)`
303303
- `render_entry_template_with_locator_without_state(template, locator, config)`
304304

305-
The `*_without_state` APIs render with an empty object root state. In WASM, the state parameter is optional for `render` and `render_with_templates`; omitted state also uses an empty object. `render_with_templates` accepts an `attribute_name_strategy` string parameter (`""`, `"none"`, or `"camelCase"`):
305+
The `*_without_state` APIs render with an empty object root state. In WASM, the state parameter is optional for `render`, `render_with_templates`, and `render_entry_with_templates`; omitted state also uses an empty object. The template-rendering WASM exports accept an `attribute_name_strategy` string parameter (`""`, `"none"`, or `"camelCase"`):
306306

307307
- `render_with_templates(entry, templates_json, state?, attribute_name_strategy?)`
308+
- `render_entry_with_templates(entry, templates_json, state?, attribute_name_strategy?)`
308309

309310
---
310311

@@ -461,12 +462,13 @@ Hand-rolled in `glob_match` → `match_segments` → `match_segment` → `match_
461462

462463
## WASM bindings — `wasm.rs`
463464

464-
`wasm.rs` is compiled only for the `wasm32-unknown-unknown` target (`#[cfg(target_arch = "wasm32")]`). It exposes three functions to JavaScript via `wasm-bindgen`:
465+
`wasm.rs` is compiled only for the `wasm32-unknown-unknown` target (`#[cfg(target_arch = "wasm32")]`). It exposes four functions to JavaScript via `wasm-bindgen`:
465466

466467
| Export | Signature | Description |
467468
|--------|-----------|-------------|
468469
| `render` | `(entry: &str, state?: string) → String` | Render a template with no custom elements; omitted state is `{}` |
469-
| `render_with_templates` | `(entry: &str, templates_json: &str, state?: string, attribute_name_strategy?: string) → String` | Render top-level entry HTML with a pre-built `{name: content}` templates map; omitted state is `{}` |
470+
| `render_with_templates` | `(entry: &str, templates_json: &str, state?: string, attribute_name_strategy?: string) → String` | Render a template with a pre-built `{name: content}` templates map using non-entry semantics; omitted state is `{}` |
471+
| `render_entry_with_templates` | `(entry: &str, templates_json: &str, state?: string, attribute_name_strategy?: string) → String` | Render top-level entry HTML with a pre-built `{name: content}` templates map; omitted state is `{}` |
470472
| `parse_f_templates` | `(html: &str) → String` | Parse `<f-template>` elements and return a JSON array |
471473

472474
### `parse_f_templates`
@@ -484,7 +486,11 @@ Calls `locator::parse_f_templates` (the same function used by `Locator::from_pat
484486

485487
### `render_with_templates`
486488

487-
Accepts `templates_json` as a JSON object string mapping element names to raw template strings (the inner content already extracted from `<template>`). Constructs a `Locator::from_templates` map and calls `render_entry_template_with_locator` when state is provided, or the no-state equivalent when it is omitted. Root-level custom elements therefore receive the full root state with HTML attributes overlaid, matching CLI entry rendering.
489+
Accepts `templates_json` as a JSON object string mapping element names to raw template strings (the inner content already extracted from `<template>`). Constructs a `Locator::from_templates` map and calls `render_template_with_locator` when state is provided, or the no-state equivalent when it is omitted. This preserves the original non-entry semantics for existing JS consumers.
490+
491+
### `render_entry_with_templates`
492+
493+
Accepts the same arguments as `render_with_templates`, but calls `render_entry_template_with_locator` when state is provided, or the no-state equivalent when it is omitted. Root-level custom elements therefore receive entry opening-tag handling, matching CLI entry rendering.
488494

489495
---
490496

crates/microsoft-fast-build/README.md

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -510,14 +510,16 @@ let result = render_template_with_locator(
510510
#### WASM / JavaScript API
511511

512512
```javascript
513-
const html = render_with_templates(
513+
const html = render_entry_with_templates(
514514
entry,
515515
templatesJson,
516516
stateJson,
517517
"camelCase" // or "none"
518518
);
519519
```
520520

521+
Use `render_with_templates` for the original non-entry template-rendering semantics; use `render_entry_with_templates` for top-level entry HTML rendering.
522+
521523
Passing `"none"` or `""` as the strategy uses the default behaviour.
522524

523525
---

crates/microsoft-fast-build/src/attribute.rs

Lines changed: 57 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -274,9 +274,10 @@ pub(crate) fn resolve_attribute_bindings_in_tag(
274274
result.push('<');
275275
result.push_str(&tag_name);
276276

277-
for (name, value) in parse_element_attributes(tag) {
277+
for attr in parse_element_attribute_parts(tag) {
278+
let name = attr.name;
278279
if name.starts_with('?') {
279-
if let Some(v) = value {
280+
if let Some(v) = attr.value {
280281
let trimmed = v.trim();
281282
if trimmed.starts_with("{{") && trimmed.ends_with("}}") && trimmed.len() > 4 {
282283
let expr = trimmed[2..trimmed.len() - 2].trim();
@@ -289,18 +290,19 @@ pub(crate) fn resolve_attribute_bindings_in_tag(
289290
continue;
290291
}
291292

292-
match value {
293+
match attr.value {
293294
None => {
294295
result.push(' ');
295-
result.push_str(&name);
296+
result.push_str(&attr.raw);
296297
}
297298
Some(v) if v.contains("{{") => {
298299
if let Some(resolved) = resolve_attribute_value(&v, root, loop_vars) {
299-
result.push_str(&format!(" {}=\"{}\"", name, resolved));
300+
append_quoted_attribute(&mut result, &name, &resolved);
300301
}
301302
}
302-
Some(v) => {
303-
result.push_str(&format!(" {}=\"{}\"", name, v));
303+
Some(_) => {
304+
result.push(' ');
305+
result.push_str(&attr.raw);
304306
}
305307
}
306308
}
@@ -314,6 +316,23 @@ pub(crate) fn resolve_attribute_bindings_in_tag(
314316
result
315317
}
316318

319+
pub(crate) fn append_quoted_attribute(out: &mut String, name: &str, value: &str) {
320+
out.push(' ');
321+
out.push_str(name);
322+
out.push_str("=\"");
323+
append_double_quote_escaped(out, value);
324+
out.push('"');
325+
}
326+
327+
fn append_double_quote_escaped(out: &mut String, value: &str) {
328+
for ch in value.chars() {
329+
match ch {
330+
'"' => out.push_str("&quot;"),
331+
_ => out.push(ch),
332+
}
333+
}
334+
}
335+
317336
fn resolve_attribute_value(
318337
value: &str,
319338
root: &crate::json::JsonValue,
@@ -368,18 +387,18 @@ pub(crate) fn strip_client_only_attrs(tag: &str) -> String {
368387
};
369388

370389
let mut out = format!("<{}", tag_name);
371-
for (name, value) in parse_element_attributes(tag) {
372-
if name.starts_with('@') || name.starts_with(':')
390+
for attr in parse_element_attribute_parts(tag) {
391+
let name = attr.name;
392+
if name.starts_with('@')
393+
|| name.starts_with(':')
373394
|| name.eq_ignore_ascii_case("f-ref")
374395
|| name.eq_ignore_ascii_case("f-slotted")
375396
|| name.eq_ignore_ascii_case("f-children")
376397
{
377398
continue;
378399
}
379-
match value {
380-
None => { out.push(' '); out.push_str(&name); }
381-
Some(v) => out.push_str(&format!(" {}=\"{}\"", name, v)),
382-
}
400+
out.push(' ');
401+
out.push_str(&attr.raw);
383402
}
384403

385404
if is_self_closing {
@@ -448,6 +467,19 @@ fn is_skippable_tag(name: &str, locator: Option<&crate::locator::Locator>) -> bo
448467
/// Returns `(attribute_name, Option<value>)` — `None` value means boolean attribute.
449468
/// Handles double-quoted, single-quoted, and unquoted values.
450469
pub(crate) fn parse_element_attributes(open_tag: &str) -> Vec<(String, Option<String>)> {
470+
parse_element_attribute_parts(open_tag)
471+
.into_iter()
472+
.map(|attr| (attr.name, attr.value))
473+
.collect()
474+
}
475+
476+
pub(crate) struct ParsedAttribute {
477+
pub name: String,
478+
pub value: Option<String>,
479+
pub raw: String,
480+
}
481+
482+
pub(crate) fn parse_element_attribute_parts(open_tag: &str) -> Vec<ParsedAttribute> {
451483
let mut attrs = Vec::new();
452484
let bytes = open_tag.as_bytes();
453485
let mut i = 0;
@@ -482,6 +514,7 @@ pub(crate) fn parse_element_attributes(open_tag: &str) -> Vec<(String, Option<St
482514
{
483515
i += 1;
484516
}
517+
let name_end = i;
485518
let name = open_tag[name_start..i].to_string();
486519

487520
if name.is_empty() {
@@ -497,7 +530,11 @@ pub(crate) fn parse_element_attributes(open_tag: &str) -> Vec<(String, Option<St
497530

498531
if i >= bytes.len() || bytes[i] != b'=' {
499532
// Boolean attribute — no value.
500-
attrs.push((name, None));
533+
attrs.push(ParsedAttribute {
534+
name,
535+
value: None,
536+
raw: open_tag[name_start..name_end].to_string(),
537+
});
501538
continue;
502539
}
503540

@@ -513,7 +550,11 @@ pub(crate) fn parse_element_attributes(open_tag: &str) -> Vec<(String, Option<St
513550
let (value, new_i) = parse_attribute_value(open_tag, bytes, i);
514551
i = new_i;
515552

516-
attrs.push((name, Some(value)));
553+
attrs.push(ParsedAttribute {
554+
name,
555+
value: Some(value),
556+
raw: open_tag[name_start..i].to_string(),
557+
});
517558
}
518559

519560
attrs
@@ -532,7 +573,7 @@ fn parse_attribute_value(open_tag: &str, bytes: &[u8], i: usize) -> (String, usi
532573
} else {
533574
let start = i;
534575
let mut end = i;
535-
while end < bytes.len() && !bytes[end].is_ascii_whitespace() && bytes[end] != b'>' && bytes[end] != b'/' {
576+
while end < bytes.len() && !bytes[end].is_ascii_whitespace() && bytes[end] != b'>' {
536577
end += 1;
537578
}
538579
(open_tag[start..end].to_string(), end)

crates/microsoft-fast-build/src/wasm.rs

Lines changed: 55 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,11 @@
1-
use wasm_bindgen::prelude::*;
2-
use std::collections::HashMap;
1+
use crate::config::{AttributeNameStrategy, RenderConfig};
32
use crate::{
4-
json, Locator, render_entry_template_with_locator, render_entry_with_locator_without_state,
5-
render_template, render_template_without_state,
3+
json, render_entry_template_with_locator, render_entry_template_with_locator_without_state,
4+
render_template, render_template_with_locator, render_template_with_locator_without_state,
5+
render_template_without_state, Locator,
66
};
7-
use crate::config::{RenderConfig, AttributeNameStrategy};
7+
use std::collections::HashMap;
8+
use wasm_bindgen::prelude::*;
89

910
/// Render a FAST HTML template with an optional JSON state string.
1011
/// Omitted state is treated as an empty object.
@@ -18,6 +19,34 @@ pub fn render(entry: &str, state: Option<String>) -> Result<String, JsValue> {
1819
.map_err(|e| JsValue::from_str(&e.to_string()))
1920
}
2021

22+
/// Render a FAST HTML template with custom element templates and an optional JSON state string.
23+
/// Omitted state is treated as an empty object.
24+
///
25+
/// This preserves the original non-entry rendering semantics for this export.
26+
/// Use `render_entry_with_templates` for top-level entry HTML rendering.
27+
///
28+
/// `templates_json` is a JSON object mapping element names to their HTML template strings,
29+
/// e.g. `{"my-button": "<template>...</template>"}`.
30+
/// `attribute_name_strategy` controls attribute-to-property mapping: `"camelCase"` (default)
31+
/// or `"none"`. Pass an empty string for the default.
32+
/// Returns the rendered HTML or throws a JavaScript error.
33+
#[wasm_bindgen]
34+
pub fn render_with_templates(
35+
entry: &str,
36+
templates_json: &str,
37+
state: Option<String>,
38+
attribute_name_strategy: Option<String>,
39+
) -> Result<String, JsValue> {
40+
let templates = parse_templates_map(templates_json)?;
41+
let locator = Locator::from_templates(templates);
42+
let config = build_config(attribute_name_strategy.as_deref())?;
43+
match state {
44+
Some(state) => render_template_with_locator(entry, &state, &locator, config.as_ref()),
45+
None => render_template_with_locator_without_state(entry, &locator, config.as_ref()),
46+
}
47+
.map_err(|e| JsValue::from_str(&e.to_string()))
48+
}
49+
2150
/// Render the top-level **entry HTML** with custom element templates and an optional JSON state string.
2251
/// Omitted state is treated as an empty object.
2352
/// Custom elements found at the root level of `entry` build child state from the full root
@@ -31,13 +60,18 @@ pub fn render(entry: &str, state: Option<String>) -> Result<String, JsValue> {
3160
/// or `"none"`. Pass an empty string for the default.
3261
/// Returns the rendered HTML or throws a JavaScript error.
3362
#[wasm_bindgen]
34-
pub fn render_with_templates(entry: &str, templates_json: &str, state: Option<String>, attribute_name_strategy: Option<String>) -> Result<String, JsValue> {
63+
pub fn render_entry_with_templates(
64+
entry: &str,
65+
templates_json: &str,
66+
state: Option<String>,
67+
attribute_name_strategy: Option<String>,
68+
) -> Result<String, JsValue> {
3569
let templates = parse_templates_map(templates_json)?;
3670
let locator = Locator::from_templates(templates);
3771
let config = build_config(attribute_name_strategy.as_deref())?;
3872
match state {
3973
Some(state) => render_entry_template_with_locator(entry, &state, &locator, config.as_ref()),
40-
None => render_entry_with_locator_without_state(entry, &locator, config.as_ref()),
74+
None => render_entry_template_with_locator_without_state(entry, &locator, config.as_ref()),
4175
}
4276
.map_err(|e| JsValue::from_str(&e.to_string()))
4377
}
@@ -68,29 +102,35 @@ fn escape_json_str(s: &str) -> String {
68102
let mut out = String::with_capacity(s.len());
69103
for c in s.chars() {
70104
match c {
71-
'"' => out.push_str("\\\""),
105+
'"' => out.push_str("\\\""),
72106
'\\' => out.push_str("\\\\"),
73107
'\n' => out.push_str("\\n"),
74108
'\r' => out.push_str("\\r"),
75109
'\t' => out.push_str("\\t"),
76-
c => out.push(c),
110+
c => out.push(c),
77111
}
78112
}
79113
out
80114
}
81115

82116
fn parse_templates_map(templates_json: &str) -> Result<HashMap<String, String>, JsValue> {
83-
let parsed = json::parse(templates_json)
84-
.map_err(|e| JsValue::from_str(&format!("Failed to parse templates JSON: {}", e.message)))?;
117+
let parsed = json::parse(templates_json).map_err(|e| {
118+
JsValue::from_str(&format!("Failed to parse templates JSON: {}", e.message))
119+
})?;
85120
match parsed {
86121
json::JsonValue::Object(obj) => {
87122
let mut map = HashMap::new();
88123
for (k, v) in obj {
89124
match v {
90-
json::JsonValue::String(s) => { map.insert(k, s); }
91-
_ => return Err(JsValue::from_str(&format!(
92-
"Template value for '{}' must be a string", k
93-
))),
125+
json::JsonValue::String(s) => {
126+
map.insert(k, s);
127+
}
128+
_ => {
129+
return Err(JsValue::from_str(&format!(
130+
"Template value for '{}' must be a string",
131+
k
132+
)))
133+
}
94134
}
95135
}
96136
Ok(map)

crates/microsoft-fast-build/tests/edge_cases.rs

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,40 @@ fn test_missing_regular_html_attribute_binding_is_omitted() {
5656
assert_eq!(result, "<div></div>");
5757
}
5858

59+
#[test]
60+
fn test_missing_regular_html_attribute_preserves_static_value_with_quotes() {
61+
let result = ok(r#"<div title='a "quote"' class="{{missing}}"></div>"#, "{}");
62+
assert_eq!(result, r#"<div title='a "quote"'></div>"#);
63+
}
64+
65+
#[test]
66+
fn test_unquoted_attribute_path_does_not_hide_later_binding() {
67+
let result = ok(
68+
r#"<img src=/assets/a.png alt="{{alt}}">"#,
69+
r#"{"alt": "Logo"}"#,
70+
);
71+
assert_eq!(result, r#"<img src=/assets/a.png alt="Logo">"#);
72+
}
73+
74+
#[test]
75+
fn test_bound_attribute_value_is_html_escaped() {
76+
let result = ok(
77+
r#"<div title="{{value}}"></div>"#,
78+
r#"{"value": "a \"quote\" & <tag>"}"#,
79+
);
80+
assert_eq!(result, r#"<div title="a &quot;quote&quot; &amp; &lt;tag&gt;"></div>"#);
81+
}
82+
83+
#[test]
84+
fn test_unquoted_attribute_path_with_missing_later_binding_is_omitted() {
85+
let result = ok(r#"<img src=/assets/a.png alt="{{alt}}">"#, "{}");
86+
assert_eq!(result, r#"<img src=/assets/a.png>"#);
87+
assert!(
88+
!result.contains("{{alt}}"),
89+
"binding must not remain unresolved: {result}"
90+
);
91+
}
92+
5993
#[test]
6094
fn test_missing_dot_path_regular_html_attribute_with_empty_state_is_omitted() {
6195
let result = ok(r#"<div class="{{ foo.bar }}"></div>"#, "{}");

0 commit comments

Comments
 (0)