* chore(javascript): update Jint to 4.15.3 and stop blocking on promises `Engine.Evaluate(...).UnwrapIfPromise()` blocks the calling thread while the engine's event loop drains, which is exactly the wrong thing to do inside an `async` method — an expression that awaits a .NET `Task`, such as one calling `getSecret()`, held a thread pool thread for the duration of the I/O. `Engine.EvaluateAsync` awaits the returned promise instead, and takes the cancellation token while it is at it. The Jint version is moved from 4.4.2 to 4.15.3. `EvaluateAsync` arrived in 4.14.0, but the pin lands past 4.15.2 deliberately: once expressions genuinely suspend and resume instead of draining the event loop on the calling thread, they exercise the async suspension machinery 4.15.2 corrected — an `await` on a right-hand side no longer stores the suspension sentinel, async generators and `for await...of` preserve loop iteration state across a suspension, and a suspension node is unwrapped correctly. Shipping the non-blocking change on an earlier 4.14/4.15 would enable exactly the code paths those releases fixed. One default changed along the way: since 4.14 `Interop.ArrayConversion` defaults to `LiveView`, so a CLR array reaches script as a live view over the original array rather than as a copy. That is observable — a script that sorts an array would now reorder the workflow's own array, and the value round-trips back as its original element type rather than as `object[]`. The evaluator therefore pins the previous `Copy` behaviour so the upgrade is not a behavioural change; hosts that prefer the live view can opt in through `JintOptions.ConfigureEngineOptions`. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0179sA2T7HuRfRfSc2JirFik * test(javascript): pin the array copy lane and adopt JsString.Create The array audit. The parent commit fixes `ArrayConversion` to `Copy`, because 4.14 changed the default to `LiveView` and the two differ in behaviour a script can observe. The existing tests assert what a copy *produces*, which a live view also satisfies for a value nothing mutates; asking the engine how many conversions of each kind it performed (4.15.1's interop conversion counters) pins the lane itself. The second assertion is the more interesting one: an ordinary evaluation converts no CLR array at all, because Elsa converts collection-valued variables itself in `ObjectConverterHelper` long before Jint's array lane could see them. That makes the `ArrayConversion` setting a narrow compatibility pin rather than something every evaluation depends on. `JsString.Create` (public since Jint 4.15.3) is adopted in `JsonElementConverter`, where the string case was the only one still routed through `JsValue.FromObject` — re-entering the whole conversion pipeline, the registered object converters and this one included, to arrive at the same call the number and boolean cases beside it already make directly. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016uV6H9cTntzsoKiaJRBn4f * perf(javascript): register .NET type globals lazily A fresh Jint engine is built for every expression evaluation, and roughly twenty .NET types are registered on it before the expression runs: the common types (`DateTime`, `Guid`, `TimeSpan`, …) plus every non-primitive workflow variable descriptor type. Each registration builds a `TypeReference`, which describes the type through reflection. The overwhelming majority of expressions reference none of them. `Options.AddLazyGlobal` installs a global whose value is produced by a factory the first time a script reads the name. Both type registration handlers now run on `CreatingJavaScriptEngine` — which already carries the `Jint.Options` being built — and register through it, so a type is only described if an expression actually mentions it. A script that uses `Guid.NewGuid()` still sees `Guid`; a script that uses none of them pays for none of them. Types whose name cannot be written as a JavaScript identifier are skipped while we are here, since no script can reach them. That covers constructed generic types (``IDictionary`2``, which two different variable descriptors both claimed) and array types (`Byte[]`). Moving the registrations to engine construction has a consequence worth pinning beyond the ordering: a global installed by the host through `configureEngine` is no longer overwritten by the built-in registration of the same name. The lazy global for `Guid` is already installed when that callback runs, and `Engine.SetValue` goes through `[[Set]]` on the global object, which reads the current value — running the factory once and discarding the result — before replacing the descriptor. The end state is the host value, but only the test says so. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016uV6H9cTntzsoKiaJRBn4f * perf(javascript): register the common functions lazily The same argument as the type globals, applied to the other bulk registration a fresh engine pays for. `ConfigureEngineWithCommonFunctions` installs twenty-seven functions on every engine — `getVariable`, `toJson`, `newGuid`, the base64 helpers, the deprecated GUID pair — and each `engine.SetValue(name, Delegate)` builds an interop function wrapper around the delegate: a JavaScript function object, plus the signature metadata and invoker lookups Jint resolves per delegate type. An expression such as `variables.Foo` or a comparison calls none of them. This was recorded in the PR as deferred, because a lazy version had to keep the `NonEnumerable` flag `SetValue(string, Delegate)` applies — otherwise the functions would start appearing in `Object.keys(globalThis)`, which is observable. Jint 4.15.3's `Engine.Advanced.AddLazyGlobal` takes a `PropertyFlag` and is the post-construction counterpart of the options-time API used for the type globals, so the flag is passed explicitly and the port is a line per function. The CLR delegate is now created inside the factory as well, so a function nothing reads costs one closure rather than a closure, a delegate and a wrapper. The laziness is invisible, and the tests say so rather than leaving it implied. `AddLazyGlobal` installs the property itself eagerly and defers only its value, so `in`, `hasOwnProperty` and `Object.getOwnPropertyNames` answer immediately without materialising anything; `Object.keys(globalThis)` still omits them; the descriptor still reports writable, non-enumerable and configurable; two reads of a function are the same value, which is what says the factory ran once and the result was stored rather than recomputed per read; and a script can still overwrite one. Not separately measured. The measured table in the PR predates this commit and was not re-run for it. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016uV6H9cTntzsoKiaJRBn4f * refactor(javascript): let Jint expose enums as names and type its converters `EnumToStringConverter` turned every CLR enum crossing into JavaScript into `Enum.ToString()`. Jint 4.15 does that with `Interop.EnumConversion`, so the converter goes away. It is not only fewer moving parts. The converter could only see values that crossed the interop boundary, and a constant read off a registered enum type does not: `LogPersistenceMode.Include` came back as the underlying number while the same value held in a workflow variable came back as `"Include"`, so `mode === LogPersistenceMode.Include` was always false. The built-in switch covers both directions and they now agree. Values written back to the CLR keep accepting the member name and the number, as they did before. That fix changes what an expression using a constant *numerically* produces, and the hazard worth calling out is persisted state: a workflow variable holding a number written from a constant before the upgrade no longer compares equal to that constant after it, which reaches in-flight and resumed instances rather than only new ones. Typed conversions hold in both directions, so activity inputs and typed variable reads are unaffected. Recorded in the 3.8.0 changelog. The two remaining converters register through the overload that declares the CLR types they handle. A converter that does not declare them has to be offered every value crossing the boundary, which costs the engine its compiled member-read and method-invoker lanes for every wrapped .NET object; declaring `byte[]` and `JsonElement` keeps those lanes for everything that cannot produce one. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016uV6H9cTntzsoKiaJRBn4f * perf(javascript): register only the accessors an expression names Every evaluation registers a getter and a setter for each variable in scope, a getter for each workflow input, and a getter for each (activity, output) pair in the enclosing container. The last of those is the expensive one: naming those accessors walks every node of the workflow and resolves each against the activity registry, so the cost of setting up an engine grows with the size of the workflow rather than the size of the expression. Almost no expression uses any of them. Jint reports the free identifiers of a prepared program through `Prepared<T>.ReferencedGlobals`, which is exactly the question being asked here: an identifier the expression never mentions cannot be read from it. The evaluator now prepares the script before configuring the engine — the parse was already cached, so this only reorders work — and passes the set on `EvaluatingJavaScript`. The accessor handler registers a name only if it is in the set, and skips the workflow walk entirely when no identifier of the `get{Output}From{Activity}` shape appears. The filter is sound for an expression that names an accessor the way one is meant to be named, and not for one that builds or reaches a name at run time. Four such forms are detectable and each turns the filter off. A direct `eval` call is reported as `HasDirectEvalCall`. An *indirect* `eval` call and the `Function` constructor are deliberately not flagged as direct calls — Jint reports the identifiers `eval` and `Function` in the set instead, and says so, because that is the signal a host is meant to act on. Missing that signal is not theoretical: `new Function('return getMyVariable()')()` would regress from working to a `ReferenceError`, since Function-constructed code resolves only against the global scope, and `var e = eval; e('getMyVariable()')` would fail the same way. The fourth is a reference to `globalThis`, which reaches a global without naming it. All four are pinned. What stays undetectable is reaching the global object without naming it at all — a sloppy-mode top-level `this`, or `[].constructor.constructor(…)`. An expression written that way loses the generated accessor but not the data: `getVariable(name)`, `getInput(name)` and `getOutputFrom(activityId, outputName)` are always registered and reach the same values. Note this does not replace the regular expression in `ConfigureEngineWithVariables`, which extracts the member names in `variables.Foo`. Those are not free identifiers and Jint deliberately does not report them; the set only says whether `variables` itself is referenced. The filtering is invisible from inside a script, which also makes it unprovable from there, so two of the tests hold on to the engine and assert on the globals directly. While here, the cancellation token is registered as an engine constraint. Passing it to `EvaluateAsync` only covers the awaiting part; Jint's own remarks say the parameter cannot preempt the synchronous evaluation loop and point at this constraint, so the token read as more coverage than it delivered. A cancellation constraint is amortizable, so the interpreter keeps its tight-loop fast path, and a default token registers nothing. #7891 supersedes this with the full constraint set. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016uV6H9cTntzsoKiaJRBn4f * perf(javascript): build marshalled variables in Jint's shaped representation `ConvertToJsObject` built the JavaScript object a workflow variable is copied into one `DefineOwnProperty` call at a time, with an explicit descriptor. That lands the object in Jint's per-object property dictionary, so every marshalled variable carries its own descriptors even though sibling variables and the repeated nested payloads of one document present the same key set. `JsObject.CreateFromEntries` defines the same writable, enumerable and configurable properties but builds through the shared-layout path. Both wins land inside a single evaluation — half the descriptor allocations, since the explicit descriptor caused a second one inside `ValidateAndApplyPropertyDescriptor`, and one shared layout across objects presenting the same keys. Nothing carries across evaluations: layouts are interned per engine and per-node inline caches live in per-engine handler trees that only engage on a second evaluation on the same engine, which a fresh-engine-per-evaluation host never reaches. Reaching the shared layout is silent: `CreateFromEntries` falls back to the ordinary property dictionary whenever a key or a growth guard says the layout cannot continue, and the object behaves identically either way. A test now asks the engine whether the object actually got one. `Engine.Advanced.HasSharedShape`, added in 4.15.3, is the part of that answer Jint documents as a contract — the finer-grained `GetObjectRepresentation` names an internal representation that may be renamed or subdivided in any release — and `CreateFromEntries` is one of its three documented success cases. The assertion is not vacuous: against the previous property-by-property build it is false, including for the `CreateDataProperty` variant in #7892, because that object is created through `Intrinsics.Object.Construct` rather than built as entries. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016uV6H9cTntzsoKiaJRBn4f * test(javascript): run the scripting suites with Jint's host-contract verifiers on Jint has a set of verifiers for the extension points it *trusts* — the ones it cannot afford to re-check on a hot path, so a violation is otherwise silent. They used to be compiled out of Release, which made "run your suite against a Debug Jint" the only way to reach them, and Jint ships Release-only. 4.15.3 moves them behind an AppContext switch read once at type initialization, so a host that never sets it pays nothing (the JIT folds the guards away) and a test host can turn them on against the shipped package. The one Elsa is subject to today is the object-converter type declaration this PR introduced. Registering a converter with `AddObjectConverter(converter, handledTypes)` promises the engine the converter produces values only for those types, and in exchange the compiled interop lanes are kept for every member that cannot produce one. Nothing links that promise to the converter's own `TryConvert` switch: a case added there and not added to the registration is silently skipped on exactly the members the declaration excluded, and honoured everywhere else. `ByteArrayConverter` and `JsonElementConverter` are consistent today; this is what would report it if they drifted. Elsa defines no `ObjectInstance` subclass, so the rest of the verifiers have nothing to check here yet. They are a standing guard for the day a host handler or a satellite module adds one. Wired as a module initializer, because the switch has to be set before the first use of any Jint type. Duplicated across the three suites that reference `Elsa.Expressions.JavaScript` rather than shared: `Elsa.Testing.Shared.Integration` would be the obvious home, but it is a published package, and a module initializer there would flip a process-wide switch for every external consumer of it as well. A one-line test pins that the initializer ran, since Elsa satisfies the contracts it is subject to and nothing else would notice the checks going away. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_016uV6H9cTntzsoKiaJRBn4f --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> Co-authored-by: Sipke Schoorstra <sipkeschoorstra@outlook.com>
80 lines
3.3 KiB
C#
80 lines
3.3 KiB
C#
using Elsa.Expressions.JavaScript.Contracts;
|
|
using Elsa.Expressions.Models;
|
|
using Elsa.Testing.Shared;
|
|
using Elsa.Workflows.LogPersistence;
|
|
using Jint;
|
|
using Microsoft.Extensions.DependencyInjection;
|
|
using Xunit;
|
|
using Xunit.Abstractions;
|
|
|
|
namespace Elsa.JavaScript.IntegrationTests;
|
|
|
|
/// <summary>
|
|
/// Pins how .NET enums are exposed to JavaScript: a value reaches script as its member name, and so does a
|
|
/// constant read off the registered enum type, so the two compare equal.
|
|
/// </summary>
|
|
public class EnumConversionTests
|
|
{
|
|
private readonly IServiceProvider _serviceProvider;
|
|
private readonly IJavaScriptEvaluator _evaluator;
|
|
|
|
public EnumConversionTests(ITestOutputHelper testOutputHelper)
|
|
{
|
|
_serviceProvider = new TestApplicationBuilder(testOutputHelper).Build();
|
|
_evaluator = _serviceProvider.GetRequiredService<IJavaScriptEvaluator>();
|
|
}
|
|
|
|
[Fact(DisplayName = "An enum value reaches a script as its member name")]
|
|
public async Task EnumValuesAreExposedAsTheirName()
|
|
{
|
|
Assert.Equal("Include", await EvaluateAsync("return mode;", engine => engine.SetValue("mode", LogPersistenceMode.Include)));
|
|
}
|
|
|
|
[Fact(DisplayName = "An enum value reaches a script as a string")]
|
|
public async Task EnumValuesAreStrings()
|
|
{
|
|
Assert.Equal("string", await EvaluateAsync("return typeof mode;", engine => engine.SetValue("mode", LogPersistenceMode.Include)));
|
|
}
|
|
|
|
[Fact(DisplayName = "A constant read off a registered enum type is its member name")]
|
|
public async Task EnumConstantsAreExposedAsTheirName()
|
|
{
|
|
Assert.Equal("Include", await EvaluateAsync("return LogPersistenceMode.Include;"));
|
|
}
|
|
|
|
[Fact(DisplayName = "An enum value compares equal to the constant of the same member")]
|
|
public async Task EnumValuesCompareEqualToTheirConstant()
|
|
{
|
|
// The two directions used to disagree: a value crossing the boundary became its name while a constant
|
|
// read off the registered type stayed the underlying number, so this comparison was always false.
|
|
Assert.Equal("true", await EvaluateAsync("return '' + (mode === LogPersistenceMode.Include);", engine => engine.SetValue("mode", LogPersistenceMode.Include)));
|
|
}
|
|
|
|
[Fact(DisplayName = "An enum-valued property of a .NET object reaches a script as its member name")]
|
|
public async Task EnumMembersOfWrappedObjectsAreExposedAsTheirName()
|
|
{
|
|
Assert.Equal("Exclude", await EvaluateAsync("return holder.Mode;", engine => engine.SetValue("holder", new ModeHolder { Mode = LogPersistenceMode.Exclude })));
|
|
}
|
|
|
|
[Fact(DisplayName = "A member name written back from a script converts to the enum value")]
|
|
public async Task EnumMembersAcceptTheirNameOnTheWayBack()
|
|
{
|
|
var holder = new ModeHolder();
|
|
|
|
await EvaluateAsync("holder.Mode = 'Exclude'; return '';", engine => engine.SetValue("holder", holder));
|
|
|
|
Assert.Equal(LogPersistenceMode.Exclude, holder.Mode);
|
|
}
|
|
|
|
private async Task<string?> EvaluateAsync(string script, Action<Engine>? configureEngine = null)
|
|
{
|
|
var context = new ExpressionExecutionContext(_serviceProvider, new());
|
|
return (string?)await _evaluator.EvaluateAsync(script, typeof(string), context, configureEngine: configureEngine);
|
|
}
|
|
|
|
private class ModeHolder
|
|
{
|
|
public LogPersistenceMode Mode { get; set; }
|
|
}
|
|
}
|