diff --git a/backend/src/Squidex.Domain.Apps.Core.Operations/Scripting/ContentWrapper/ContentFieldObject.cs b/backend/src/Squidex.Domain.Apps.Core.Operations/Scripting/ContentWrapper/ContentFieldObject.cs
index 5f05220a9..0095307e8 100644
--- a/backend/src/Squidex.Domain.Apps.Core.Operations/Scripting/ContentWrapper/ContentFieldObject.cs
+++ b/backend/src/Squidex.Domain.Apps.Core.Operations/Scripting/ContentWrapper/ContentFieldObject.cs
@@ -134,10 +134,10 @@ public sealed class ContentFieldObject : ObjectInstance
protected override OwnPropertyProbe ProbeOwnProperty(JsValue property)
{
- // Deliberately mirrors GetOwnProperty above, minus the descriptor: the flags are on the descriptor
- // itself, so an existence or enumerability question is answered without ever reading CustomValue,
- // which is what maps the JSON value to a JsValue. The engine trusts the answer without verifying it,
- // so the two must stay in step.
+ // Answers whether a key exists without converting its value, which reading the property would do.
+ // Used by "in", hasOwnProperty, Object.keys, spread and JSON.stringify. Must give the same answer as
+ // GetOwnProperty above, which Jint does not check at runtime, only in tests (see
+ // JintHostContractVerification), so the two methods are kept identical apart from the return value.
EnsurePropertiesInitialized();
var propertyName = property.AsString();
diff --git a/backend/src/Squidex.Domain.Apps.Core.Operations/Scripting/Internal/JintExtensions.cs b/backend/src/Squidex.Domain.Apps.Core.Operations/Scripting/Internal/JintExtensions.cs
index 2dc19097d..b25920d26 100644
--- a/backend/src/Squidex.Domain.Apps.Core.Operations/Scripting/Internal/JintExtensions.cs
+++ b/backend/src/Squidex.Domain.Apps.Core.Operations/Scripting/Internal/JintExtensions.cs
@@ -74,9 +74,8 @@ public static class JintExtensions
{
foreach (var (key, item) in vars)
{
- // Deferred instead of Engine.SetValue, which maps every variable now. The global itself is
- // installed eagerly, so existence checks and enumeration see the name without materializing
- // anything; only the mapping waits for the first read of the value.
+ // Sets the value, but runs the conversion only when the script reads it for the first time.
+ // The name is added right away, so enumeration and "in" checks work as before.
engine.Advanced.AddLazyGlobal(key, e => MapVariable(e, item));
}
}
@@ -87,8 +86,8 @@ public static class JintExtensions
}
///
- /// The conversion performs, including its special case for
- /// a CLR type, so deferring a variable cannot change what the script sees.
+ /// Converts a value exactly like does, including its
+ /// special case for types, so that a deferred variable cannot look different from an eager one.
///
private static JsValue MapVariable(Engine engine, object? item)
{
diff --git a/backend/src/Squidex.Domain.Apps.Core.Operations/Scripting/Internal/JintObjectConverter.cs b/backend/src/Squidex.Domain.Apps.Core.Operations/Scripting/Internal/JintObjectConverter.cs
index e9dfd69ee..ac0cfad60 100644
--- a/backend/src/Squidex.Domain.Apps.Core.Operations/Scripting/Internal/JintObjectConverter.cs
+++ b/backend/src/Squidex.Domain.Apps.Core.Operations/Scripting/Internal/JintObjectConverter.cs
@@ -22,14 +22,14 @@ namespace Squidex.Domain.Apps.Core.Scripting.Internal;
public sealed class JintObjectConverter : IObjectConverter
{
///
- /// The CLR types this converter answers for, declared at registration so the engine can keep its
- /// compiled interop member-read lane for members whose declared type can never reach this converter.
+ /// The types this converter handles, passed to Jint when the converter is registered.
///
///
- /// Matching is by assignability, so covers every implementation. Registering the
- /// converter without this set makes every wrapped CLR member read in the engine take the slow lane.
- /// Enums are not listed: they are handled natively through
- /// .
+ /// Without this list Jint has to offer every property of every .NET object to this converter and cannot
+ /// use its faster property reader for any of them. Base types and interfaces count, so
+ /// covers all implementations. Keep the list in sync with the switch below - a type
+ /// that is converted but not listed here fails a test (see JintHostContractVerification). Enums are
+ /// missing on purpose, they are converted by Jint itself, see EnumConversion in JintScriptEngine.
///
public static readonly Type[] HandledTypes =
[
diff --git a/backend/src/Squidex.Domain.Apps.Core.Operations/Scripting/Internal/JsonMapper.cs b/backend/src/Squidex.Domain.Apps.Core.Operations/Scripting/Internal/JsonMapper.cs
index 3f0ed8ac1..00e9e7e56 100644
--- a/backend/src/Squidex.Domain.Apps.Core.Operations/Scripting/Internal/JsonMapper.cs
+++ b/backend/src/Squidex.Domain.Apps.Core.Operations/Scripting/Internal/JsonMapper.cs
@@ -55,9 +55,9 @@ public static class JsonMapper
private static JsObject FromObject(JsonObject obj, Engine engine)
{
- // Built through the hidden class machinery, so JSON objects sharing a key sequence - every content
- // item of the same schema does - share one hidden class and keep a script reading them monomorphic.
- // A bare ObjectInstance subclass can never be in shape mode and is outside the read caches entirely.
+ // Objects that are created this way and have the same keys - all content items of a schema do -
+ // share one description of their layout, like a class. Reading a property is then a lot faster than
+ // with a custom ObjectInstance class, where every single object gets its own property dictionary.
var entries = new KeyValuePair[obj.Count];
var index = 0;
@@ -119,8 +119,8 @@ public static class JsonMapper
var result = new JsonArray((int)length);
- // The indexed accessor reads the dense backing directly, where a string key would allocate one
- // key per element and route through the full property lookup.
+ // The indexer reads the array storage directly. The old version converted the index to a string
+ // and did a full property lookup for every element.
for (var i = 0u; i < length; i++)
{
result.Add(Map(a[i]));
diff --git a/backend/src/Squidex.Domain.Apps.Core.Operations/Scripting/JintScriptEngine.cs b/backend/src/Squidex.Domain.Apps.Core.Operations/Scripting/JintScriptEngine.cs
index faf8c20cc..0181c43ad 100644
--- a/backend/src/Squidex.Domain.Apps.Core.Operations/Scripting/JintScriptEngine.cs
+++ b/backend/src/Squidex.Domain.Apps.Core.Operations/Scripting/JintScriptEngine.cs
@@ -145,7 +145,10 @@ public sealed class JintScriptEngine(IMemoryCache cache, IOptions new CustomClrConverter(engine));
engineOptions.SetReferencesResolver(NullPropagation.Instance, NullPropagation.Interests);
engineOptions.Strict();
diff --git a/backend/src/Squidex.Domain.Apps.Core.Operations/Scripting/NullPropagation.cs b/backend/src/Squidex.Domain.Apps.Core.Operations/Scripting/NullPropagation.cs
index 8e8d0c724..73cd0a536 100644
--- a/backend/src/Squidex.Domain.Apps.Core.Operations/Scripting/NullPropagation.cs
+++ b/backend/src/Squidex.Domain.Apps.Core.Operations/Scripting/NullPropagation.cs
@@ -15,17 +15,13 @@ namespace Squidex.Domain.Apps.Core.Scripting;
public sealed class NullPropagation : IReferenceResolver
{
///
- /// The situations this resolver actually answers, declared so the engine keeps the fast paths for
- /// everything else.
+ /// The cases this resolver actually handles.
///
///
- /// Deliberately omitted are and
- /// , the pair that disables the
- /// non-computed member-read inline caches, the dense-array indexed-read lane and the member-call callee
- /// lane engine-wide. declines every base that is not null or
- /// undefined, so those are situations where the engine consulting this resolver could never change the
- /// result. Interests are a subscription filter and not a promise: a situation not subscribed to behaves
- /// exactly as if no resolver were registered.
+ /// Without this list Jint has to assume that we want to see every property read and turns off its read
+ /// caches for the whole engine. But only ever does something when the
+ /// value is null or undefined, so the other cases can be left to Jint. Behavior does not change: for a
+ /// case that is not listed here Jint behaves as if no resolver was registered at all.
///
public const ReferenceResolverInterests Interests =
ReferenceResolverInterests.NullishPropertyBase |
@@ -35,16 +31,12 @@ public sealed class NullPropagation : IReferenceResolver
public static readonly NullPropagation Instance = new NullPropagation();
///
- /// Answers a read of a name that resolves to no binding, so that an unknown name does not throw a
- /// reference error.
+ /// Called when a name does not exist, so that reading an unknown variable does not throw.
///
///
- /// Passing the reference base straight through hands script the engine's internal sentinel for the
- /// unresolvable state - a reading [[Unresolvable]] - rather than
- /// undefined, which is documented on and
- /// on . That is what scripts have always seen here, so it is kept and pinned
- /// by a test; assigning instead would be the tidier behaviour but a
- /// breaking change for existing tenant scripts.
+ /// The returned base is not undefined here but an internal Jint marker string that reads
+ /// [[Unresolvable]]. That is what scripts have always seen, so it is kept as it is and covered by
+ /// a test. Returning undefined would be nicer, but would change behavior for existing scripts.
///
public bool TryUnresolvableReference(Engine engine, Reference reference, out JsValue value)
{
diff --git a/backend/src/Squidex.Domain.Apps.Core.Operations/Scripting/WritableContext.cs b/backend/src/Squidex.Domain.Apps.Core.Operations/Scripting/WritableContext.cs
index 3ccfcab70..4c248a78e 100644
--- a/backend/src/Squidex.Domain.Apps.Core.Operations/Scripting/WritableContext.cs
+++ b/backend/src/Squidex.Domain.Apps.Core.Operations/Scripting/WritableContext.cs
@@ -21,12 +21,10 @@ internal sealed class WritableContext : ObjectInstance
{
this.vars = vars;
- // Scripts touch a fraction of the variables, but mapping one is not always cheap: a content data
- // variable builds a wrapper, a user variable walks and groups every claim. The descriptors are
- // installed eagerly - so key order, enumeration and existence checks are exactly what they were -
- // and only the mapping waits for the first read of a value. Once it has run the descriptor drops
- // back to an ordinary data property and rejoins the write inline cache, which is what a
- // hand-written CustomJsValue descriptor cannot do.
+ // Adds the value, but runs the conversion only when the script reads it for the first time. Most
+ // scripts use a few of these variables and some of them are expensive, e.g. the user variable walks
+ // and groups all claims. The properties themselves are added right away, so key order, enumeration
+ // and "in" checks stay the same.
foreach (var (key, item) in vars)
{
SetOwnProperty(key, PropertyDescriptor.CreateLazy(
diff --git a/backend/tests/Squidex.Domain.Apps.Core.Tests/Operations/Scripting/JintScriptEngineTests.cs b/backend/tests/Squidex.Domain.Apps.Core.Tests/Operations/Scripting/JintScriptEngineTests.cs
index fec93fd7b..5ed63d5e2 100644
--- a/backend/tests/Squidex.Domain.Apps.Core.Tests/Operations/Scripting/JintScriptEngineTests.cs
+++ b/backend/tests/Squidex.Domain.Apps.Core.Tests/Operations/Scripting/JintScriptEngineTests.cs
@@ -729,9 +729,8 @@ public class JintScriptEngineTests : IClassFixture
[Fact]
public void Should_not_throw_if_reading_undeclared_identifier()
{
- // The null propagation resolver answers an unresolvable reference with the reference base, which is
- // Jint's internal sentinel. The value is odd, but it is what scripts have always seen and the point
- // of the test is that the read does not throw a reference error.
+ // Reading an unknown name does not throw, it returns an internal Jint marker string. That is odd,
+ // but it is what scripts have always seen here, see NullPropagation.TryUnresolvableReference.
const string script = @"
String(unknownName) + '|' + (typeof unknownName);
";
diff --git a/backend/tests/Squidex.Domain.Apps.Core.Tests/Operations/Scripting/JsonMapperTests.cs b/backend/tests/Squidex.Domain.Apps.Core.Tests/Operations/Scripting/JsonMapperTests.cs
index 2872ebcd8..40ce7b1b4 100644
--- a/backend/tests/Squidex.Domain.Apps.Core.Tests/Operations/Scripting/JsonMapperTests.cs
+++ b/backend/tests/Squidex.Domain.Apps.Core.Tests/Operations/Scripting/JsonMapperTests.cs
@@ -22,10 +22,9 @@ public class JsonMapperTests
var mapped = (ObjectInstance)JsonMapper.Map(CreateJson(), engine);
var nested = (ObjectInstance)mapped.Get("nested");
- // A shared shape is what keeps a script reading a batch of content items monomorphic. It is a
- // performance property and never a correctness one, but it is silent when it regresses: building
- // these objects as a host ObjectInstance subclass again would put them back in the per-object
- // dictionary with no test noticing.
+ // Sharing the layout is what makes reading properties of many content items fast. It only affects
+ // performance and never behavior, which is why it is asserted here: building these objects with a
+ // custom ObjectInstance class again would silently undo it and no other test would notice.
Assert.True(engine.Advanced.HasSharedShape(mapped));
Assert.True(engine.Advanced.HasSharedShape(nested));
}
diff --git a/backend/tests/Squidex.Domain.Apps.Core.Tests/TestHelpers/JintHostContractVerification.cs b/backend/tests/Squidex.Domain.Apps.Core.Tests/TestHelpers/JintHostContractVerification.cs
index 3fe8ea3b0..6bcce4278 100644
--- a/backend/tests/Squidex.Domain.Apps.Core.Tests/TestHelpers/JintHostContractVerification.cs
+++ b/backend/tests/Squidex.Domain.Apps.Core.Tests/TestHelpers/JintHostContractVerification.cs
@@ -10,19 +10,17 @@ using System.Runtime.CompilerServices;
namespace Squidex.Domain.Apps.Core.TestHelpers;
///
-/// Turns on Jint's host-contract verifiers for this test assembly.
+/// Turns on Jint's self checks for this test assembly.
///
///
-/// The scripting integration defines several Jint extension points - the ContentWrapper objects override
-/// GetOwnProperty and ProbeOwnProperty, and the engine trusts both without re-verifying them on the hot
-/// path. A hook that contradicts another therefore fails silently in production: a key vanishes from every
-/// enumeration, or a read resolves on the prototype for a property that exists. With the switch on, Jint
-/// recomputes the answer the fast paths exist to avoid and throws on the first disagreement, so these tests
-/// are the checker.
+/// Our ContentWrapper classes and the object converter implement Jint extension points where Jint relies on
+/// our answers being consistent, without checking them - checking would cost as much as the shortcut saves.
+/// A mistake there is silent in production: a key can disappear from Object.keys, or a converted type can be
+/// skipped. With this switch on Jint verifies the answers and throws on the first mismatch, so a mistake
+/// fails a test instead.
///
-/// It has to be set before the first use of any Jint type - the flag is read once at type initialization -
-/// which is what the module initializer guarantees. Never turn it on in production: the verifiers
-/// deliberately redo the work they check.
+/// The switch has to be set before the first Jint type is used, which is what the module initializer
+/// guarantees. It stays off in production, where the checks would only cost performance.
///
///
internal static class JintHostContractVerification