Skip to content

Add JsonNode support & enhance JsonElement - #670

Open
jsneedles wants to merge 6 commits into
Handlebars-Net:masterfrom
jsneedles:feature/json-node-support
Open

jsneedles wants to merge 6 commits into
Handlebars-Net:masterfrom
jsneedles:feature/json-node-support

Conversation

@jsneedles

Copy link
Copy Markdown

Hello @rexm, wanted to say how much I love this lib. We use it extensively in HTML/CSS to Image.

As a follow-up to #657, this adds support for passing JsonNode (JsonObject / JsonArray / JsonValue) directly into templates, with the same behavior as JsonElement: member access, #each with @index/@key/@first/@last, truthiness, includeZero, and partials.

It also expands JsonElement:

  • Array index access ({{items.[0]}}, {{lookup items 1}})
  • {{items.Count}} on arrays, plus {{items.length}} when UseCollectionMemberAliasProvider() is enabled (same as other collections)
  • Custom alias providers are now consulted (e.g. AddAlias<JsonElement>(...))

A JsonValue built in code that wraps a .NET collection or object (e.g. JsonValue.Create(new List<string>())) behaves like the wrapped value: it can be iterated, its members accessed, and it can be passed to partials. Members resolve by .NET property name, so [JsonPropertyName] and naming policies don't apply. The README documents this.

I moved the existing JsonElement tests out of DynamicTests into a new JsonTests file, where a shared base class runs the same cases against both JsonElement and JsonNode.

Public API additions are non-breaking: JsonNodeObjectDescriptorProvider, JsonNodeMemberAccessor, JsonNodeIterator, and new constructor overloads on JsonElementObjectDescriptorProvider / JsonElementMemberAccessor that take alias providers (the parameterless ones remain).

A few questions:

  1. System.Text.Json version. The netstandard targets pin STJ 8.0.5 (net8.0+ use the in-box version). .NET 8 / STJ 8.x go out of support on Nov 10, 2026, and 9+ also caches JsonNode.GetValueKind(), which the wrapped-value path uses. Bumping the netstandard reference to 10.x would mean .NET Framework consumers take STJ 10 and its dependencies. Worth doing, here or in a separate PR?

  2. Output formatting. {{value}} output differs slightly between the two types: a JsonNode bool renders true (its JSON form), while a JsonElement bool renders True like any other .NET bool. Likewise, a JsonValue built in code that holds a DateTime renders quoted, because it stringifies to JSON. I could add a formatter that unwraps JsonValue to its underlying .NET value so both types match normal Handlebars.Net output (True, DateTimeFormatter dates). That's also what Handlebars.Net.Extension.Json does, since it converts JSON values to .NET values. Or we could keep JSON-style output. Preference?

fix: resolve block params named like @-variables by name

Unrelated to the JsonNode changes in this PR. The new JSON array iteration tests use `as |item index|`, which exposed this bug as an intermittent CI failure.

A block param that shares a name with a context variable (e.g. `{{#each items as |item index|}}`) was looked up in BlockParamsObject using the entry index of the matching @-variable in ContextDataObject. The two dictionaries usually place the key in the same slot, so this mostly worked, but when a hash collision moved the @-variable's slot, the lookup returned another block param's value or nothing. String hashes are randomized per process, so the failure was intermittent on every OS.

Look block params up by segment, as TryGetContextVariable already does. Adds a regression test covering index, key, first, last, root and parent. Because the bug depends on the process hash seed, the test cannot fail deterministically without the fix.
@rexm

rexm commented Oct 17, 2026

Copy link
Copy Markdown
Member

Thanks for this @jsneedles! The truthiness fix for JsonNode and the new JsonElement array access are both useful, and having the same tests run against both types is great. Below are some question and suggestions:

1. Should the block param fix be its own PR?
Suggested: yes. The fix is correct, but it's unrelated to JSON and worth shipping as a patch release by itself. It's also easier to review and revert on its own.

2. Can a wrapped JsonValue avoid GetValueKind()?
GetValueKind() serializes the wrapped value. In cases like these, it would now throw an exception where we currently render empty: 1) an object with a reference cycle → JsonException: A possible object cycle was detected; 2) JsonValue.Create(typeof(string)) → NotSupportedException
It would also fail in trimmed/AOT apps with reflection serialization off. On netstandard (STJ 8.x) it reserializes on every member access.
Suggested: use wrapped.TryGetValue(out var v). A JsonElement goes to the JsonElement path, a non-string collection or object goes to its own descriptor, and everything else is a scalar.

3. Should wrapped JsonValue support be in this PR at all?
Resolving members by .NET names inside a JSON tree, so [JsonPropertyName] is ignored, is a new contract we'd be committing to.
Suggested: Consider dropping it for now and adding it later if people ask.

4. Is the JsonArray special case needed?
JsonArray already works through the existing list descriptor. On master, #each gives the same @index/@first/@last output as this branch.
Suggested: drop the JsonArray branch and JsonNodeIterator.IterateArray, and keep only the JsonObject/JsonValue handling.

5. Should JsonObject use alias providers?
AddAlias and AddAlias work, but AddAlias doesn't.
Suggested: yes. Pass alias providers into JsonNodeMemberAccessor the same way you did for JsonElementMemberAccessor.

6. Can the JsonNodeObjectDescriptorProvider constructor take alias providers instead of EnumerableObjectDescriptor?
Taking EnumerableObjectDescriptor exposes internals through the public API, and it doesn't match the JsonElement provider.
Suggested: take IReadOnlyList (plus a parameterless overload), like JsonElementObjectDescriptorProvider. If #4 is done, it may not need anything else.

7. Should JsonArray hide Parent, Root and Options?
These resolve through reflection, so {{tags.Parent}} prints the whole parent node. JsonObject already hides them today.
Suggested: it's fine to leave for a follow-up, but if you're already in there, only exposing index access and Count would be cleaner.

8. Your question on the STJ version
Suggested: a separate PR. It changes dependencies for .NET Framework users, so it needs its own release note. We'd like to do it before 8.x goes out of support in November.

9. Your question on output formatting (true vs True)
Suggested: keep this PR free of output changes. JsonNode already renders false on master, so a new formatter would change existing output. Barring exceptional circumstances, we have been careful to preserve existing quirks since this library is so widely used. For now, a note in the README about the difference would help.

For the release notes, these change what existing users see, so we'll call them out:

  • #if on a JsonNode false, 0 or "" is now falsy (it was truthy before)
  • JsonObject lookups now fall back to the lowercase name
  • custom alias providers are now called for JsonElement

Thanks again!

@jsneedles

Copy link
Copy Markdown
Author

Thanks for the thorough review @rexm

I’ve narrowed the changes accordingly:

  1. Removed the block-param fix from this PR and split it out separately to fix: resolve block parameters named like context variables #671. The JSON tests now use block-param names that don’t depend on that fix.
  2. Removed the GetValueKind() calls from the JsonNode descriptor, accessor, and iterator paths. Scalar truthiness reads the underlying value without serialization.
  3. Dropped member access and iteration for CLR objects/collections wrapped in JsonValue. Happy to revisit later.
  4. Removed the custom JsonArray handling and IterateArray; arrays use the existing list descriptor and iterator.
  5. Added alias-provider support for JsonObject, preserving exact-name and lowercase JSON property lookup before consulting aliases.
  6. Changed the JsonNode provider constructor to accept IReadOnlyList, with a parameterless overload matching the JsonElement provider.
  7. Restricted JsonArray member access to indexes, Count, and configured aliases. Parent, Root, and Options no longer resolve through reflection. It still uses the existing list iterator. Added regression coverage with collection aliases enabled and disabled.
  8. Left the STJ dependency unchanged.
  9. Kept output formatting unchanged and documented the existing boolean formatting difference in the README, along with the compatibility notes.

The shared JsonElement/JsonNode tests remain, with additional alias and non-serialization regressions.

All tests pass, and all four library targets build.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants