From 0245d4fa158e7e55914caed11ae7f42053915bda Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Standa=20Luke=C5=A1?= Date: Sat, 26 Apr 2025 13:37:37 +0200 Subject: [PATCH] Fix default values of PostbackScriptOptions Resolves #1908 We now store all options as nullable, and the default value is determined by the KnockoutHelper function called. It is necessary, as GenerateClientPostbackLambda has a different set of default values than GenerateClientPostbackLambda, and GenerateClientPostbackScript. Notable exception is the ReturnValue, which already is nullable and null means that `null` should be returned. It's thus technically difficult to make this property "optional", and this issue isn't really relevant for this property anyway --- .../Javascript/ParametrizedCode.cs | 1 + .../Framework/Controls/KnockoutHelper.cs | 15 ++--- .../Controls/PostbackScriptOptions.cs | 63 +++++++++++++------ .../Binding/JavascriptCompilationTests.cs | 1 - src/Tests/Binding/KnockoutHelperTests.cs | 62 ++++++++++++++++++ 5 files changed, 116 insertions(+), 26 deletions(-) create mode 100644 src/Tests/Binding/KnockoutHelperTests.cs diff --git a/src/Framework/Framework/Compilation/Javascript/ParametrizedCode.cs b/src/Framework/Framework/Compilation/Javascript/ParametrizedCode.cs index 3488effbd5..1450733229 100644 --- a/src/Framework/Framework/Compilation/Javascript/ParametrizedCode.cs +++ b/src/Framework/Framework/Compilation/Javascript/ParametrizedCode.cs @@ -303,6 +303,7 @@ public void Add(CodeParameterInfo parameter) public void Add(ParametrizedCode code, byte operatorPrecedence = 20) { + ThrowHelpers.ArgumentNull(code); var needsParens = code.OperatorPrecedence.NeedsParens(operatorPrecedence); if (needsParens) Add("("); code.CopyTo(this); diff --git a/src/Framework/Framework/Controls/KnockoutHelper.cs b/src/Framework/Framework/Controls/KnockoutHelper.cs index 48416b83fd..a5c9b8c157 100644 --- a/src/Framework/Framework/Controls/KnockoutHelper.cs +++ b/src/Framework/Framework/Controls/KnockoutHelper.cs @@ -116,7 +116,7 @@ public static void AddKnockoutForeachDataBind(this IHtmlWriter writer, string ex /// Generates a function expression that invokes the command with specified commandArguments. Creates code like `(...commandArguments) => dotvvm.postBack(...)` public static string GenerateClientPostbackLambda(string propertyName, ICommandBinding command, DotvvmBindableObject control, PostbackScriptOptions? options = null) { - options ??= PostbackScriptOptions.KnockoutBinding; + options = PostbackScriptOptions.KnockoutBinding.Override(options); var addArguments = options.CommandArgs is null && (command is IStaticCommandBinding || command.CommandJavascript.EnumerateAllParameters().Any(p => p == CommandBindingExpression.CommandArgumentsParameter)); if (addArguments) @@ -183,7 +183,7 @@ string getContextPath(DotvvmBindableObject? current) string getHandlerScript() { - if (!options.AllowPostbackHandlers) return "[]"; + if (options.AllowPostbackHandlers == false) return "[]"; // turn validation off for static commands var validationPathExpr = expression is IStaticCommandBinding ? null : GetValidationTargetExpression(control); return GetPostBackHandlersScript(control, propertyName, @@ -194,8 +194,8 @@ string getHandlerScript() $"[\"validate\", {{fn:{validationPathExpr.Value.javascriptExpression}, path:{MakeStringLiteral(validationPathExpr.Value.identificationExpression)}}}]", // use window.setTimeout - options.UseWindowSetTimeout ? "\"timeout\"" : null, - options.IsOnChange ? "\"suppressOnUpdating\"" : null, + options.UseWindowSetTimeout == true ? "\"timeout\"" : null, + options.IsOnChange == true ? "\"suppressOnUpdating\"" : null, GenerateConcurrencyModeHandler(propertyName, control) ); } @@ -212,13 +212,14 @@ string getHandlerScript() adjustedExpression = adjustedExpression.AssignParameters(options.ParameterAssignment); } // when the expression changes the dataContext, we need to override the default knockout context fo the command binding. + var elementAccessor = options.ElementAccessor ?? CodeParameterAssignment.FromIdentifier("this"); CodeParameterAssignment knockoutContext; CodeParameterAssignment viewModel = default; if (!isStaticCommand) { knockoutContext = options.KoContext ?? ( // adjustedExpression != expression.CommandJavascript ? - new CodeParameterAssignment(new ParametrizedCode.Builder { "ko.contextFor(", options.ElementAccessor.Code!, ")" }.Build(OperatorPrecedence.Max)) + new CodeParameterAssignment(new ParametrizedCode.Builder { "ko.contextFor(", elementAccessor.Code!, ")" }.Build(OperatorPrecedence.Max)) ); viewModel = JavascriptTranslator.KnockoutViewModelParameter.DefaultAssignment.Code; } @@ -240,7 +241,7 @@ options.KoContext is object ? { var commandArgsString = (options.CommandArgs?.Code != null) ? SubstituteArguments(options.CommandArgs!.Value.Code!) : "[]"; var args = new List { - SubstituteArguments(options.ElementAccessor.Code!), + SubstituteArguments(elementAccessor.Code!), getHandlerScript(), commandArgsString, optionalKnockoutContext.Code?.Apply(SubstituteArguments) ?? "undefined", @@ -270,7 +271,7 @@ options.KoContext is object ? string SubstituteArguments(ParametrizedCode parametrizedCode) { return parametrizedCode.ToString(p => - p == JavascriptTranslator.CurrentElementParameter ? options.ElementAccessor : + p == JavascriptTranslator.CurrentElementParameter ? elementAccessor : p == CommandBindingExpression.CurrentPathParameter ? CodeParameterAssignment.FromIdentifier(getContextPath(control)) : p == CommandBindingExpression.ControlUniqueIdParameter ? uniqueControlId?.GetParametrizedJsExpression(control) ?? CodeParameterAssignment.FromLiteral("") : p == JavascriptTranslator.KnockoutContextParameter ? knockoutContext : diff --git a/src/Framework/Framework/Controls/PostbackScriptOptions.cs b/src/Framework/Framework/Controls/PostbackScriptOptions.cs index 2a77dd9cb4..b1b0ae161b 100644 --- a/src/Framework/Framework/Controls/PostbackScriptOptions.cs +++ b/src/Framework/Framework/Controls/PostbackScriptOptions.cs @@ -8,19 +8,20 @@ namespace DotVVM.Framework.Controls /// Options for the method. public sealed record PostbackScriptOptions { - /// If true, the command invocation will be wrapped in window.setTimeout with timeout 0. This is necessary for some event handlers, when the handler is invoked before the change is actually applied. - public bool UseWindowSetTimeout { get; init; } - /// Return value of the event handler. If set to false, the script will also include event.stopPropagation() + /// If true, the command invocation will be wrapped in window.setTimeout with timeout 0. This is necessary for some event handlers, when the handler is invoked before the change is actually applied. Default is false + public bool? UseWindowSetTimeout { get; init; } + /// Return value of the event handler. If set to false, the script will also include event.stopPropagation(). Null means that `null` will be returned. public bool? ReturnValue { get; init; } - public bool IsOnChange { get; init; } + /// When true, the invocation is suppressed while viewmodel is being updated. Default is false. + public bool? IsOnChange { get; init; } /// Javascript variable where the sender element can be found. Set to $element when in knockout binding. - public CodeParameterAssignment ElementAccessor { get; init; } + public CodeParameterAssignment? ElementAccessor { get; init; } /// Javascript variable current knockout binding context can be found. By default, `ko.contextFor({elementAccessor})` is used public CodeParameterAssignment? KoContext { get; init; } /// Javascript expression returning an array of command arguments. public CodeParameterAssignment? CommandArgs { get; init; } - /// When set to false, postback handlers will not be invoked for this command. - public bool AllowPostbackHandlers { get; init; } + /// When set to false, postback handlers will not be invoked for this command. Default is true. + public bool? AllowPostbackHandlers { get; init; } /// Javascript expression returning AbortSignal which can be used to cancel the postback (it's a JS variant of CancellationToken). public CodeParameterAssignment? AbortSignal { get; init; } public Func? ParameterAssignment { get; init; } @@ -28,25 +29,26 @@ public sealed record PostbackScriptOptions /// If true, the command invocation will be wrapped in window.setTimeout with timeout 0. This is necessary for some event handlers, when the handler is invoked before the change is actually applied. /// Return value of the event handler. If set to false, the script will also include event.stopPropagation() /// If set to true, the command will be suppressed during updating of view model. This is necessary for certain onChange events, if we don't want to trigger the command when the view model changes. - /// Javascript variable where the sender element can be found. Set to $element when in knockout binding. + /// Javascript variable where the sender element can be found. Set to $element when in knockout binding, and this when in JS event. /// Javascript variable current knockout binding context can be found. By default, `ko.contextFor({elementAccessor})` is used /// Javascript expression returning an array of command arguments. /// When set to false, postback handlers will not be invoked for this command. /// Javascript expression returning AbortSignal which can be used to cancel the postback (it's a JS variant of CancellationToken). - public PostbackScriptOptions(bool useWindowSetTimeout = false, + public PostbackScriptOptions( + bool? useWindowSetTimeout = null, bool? returnValue = false, - bool isOnChange = false, - string elementAccessor = "this", + bool? isOnChange = null, + string? elementAccessor = null, CodeParameterAssignment? koContext = null, CodeParameterAssignment? commandArgs = null, - bool allowPostbackHandlers = true, + bool? allowPostbackHandlers = null, CodeParameterAssignment? abortSignal = null, Func? parameterAssignment = null) { this.UseWindowSetTimeout = useWindowSetTimeout; this.ReturnValue = returnValue; this.IsOnChange = isOnChange; - this.ElementAccessor = new CodeParameterAssignment(elementAccessor, OperatorPrecedence.Max); + this.ElementAccessor = elementAccessor is null ? (CodeParameterAssignment?)null : new CodeParameterAssignment(elementAccessor, OperatorPrecedence.Max); this.KoContext = koContext; this.CommandArgs = commandArgs; this.AllowPostbackHandlers = allowPostbackHandlers; @@ -55,19 +57,44 @@ public PostbackScriptOptions(bool useWindowSetTimeout = false, } /// Default postback options, optimal for placing the script into a `onxxx` event attribute. - public static readonly PostbackScriptOptions JsEvent = new PostbackScriptOptions(); - public static readonly PostbackScriptOptions KnockoutBinding = new PostbackScriptOptions(elementAccessor: "$element", koContext: new CodeParameterAssignment("$context", OperatorPrecedence.Max, isGlobalContext: true)); + public static readonly PostbackScriptOptions JsEvent = new PostbackScriptOptions( + elementAccessor: "this", + koContext: new CodeParameterAssignment(new ParametrizedCode(["ko.contextFor(", ")"], [JavascriptTranslator.CurrentElementParameter.ToInfo()], OperatorPrecedence.Max)) + ); + public static readonly PostbackScriptOptions KnockoutBinding = new PostbackScriptOptions( + elementAccessor: "$element", + koContext: new CodeParameterAssignment("$context", OperatorPrecedence.Max, isGlobalContext: true) + ); + + public PostbackScriptOptions WithDefaults(PostbackScriptOptions? defaults) + { + if (defaults is null) return this; + return this with { + UseWindowSetTimeout = UseWindowSetTimeout ?? defaults.UseWindowSetTimeout, + // ReturnValue is ignored on purpose, because it is set to false in the constructor + IsOnChange = IsOnChange ?? defaults.IsOnChange, + ElementAccessor = ElementAccessor ?? defaults.ElementAccessor, + KoContext = KoContext ?? defaults.KoContext, + CommandArgs = CommandArgs ?? defaults.CommandArgs, + AllowPostbackHandlers = AllowPostbackHandlers ?? defaults.AllowPostbackHandlers, + AbortSignal = AbortSignal ?? defaults.AbortSignal, + ParameterAssignment = ParameterAssignment ?? defaults.ParameterAssignment, + }; + } + + public PostbackScriptOptions Override(PostbackScriptOptions? overrides) => + overrides is null ? this : overrides.WithDefaults(this); public override string ToString() { var fields = new List(); - if (UseWindowSetTimeout) fields.Add("useWindowSetTimeout: true"); + if (UseWindowSetTimeout is {}) fields.Add($"useWindowSetTimeout: {UseWindowSetTimeout}"); if (ReturnValue != false) fields.Add($"returnValue: {(ReturnValue == true ? "true" : "null")}"); - if (IsOnChange) fields.Add("isOnChange: true"); + if (IsOnChange is {}) fields.Add($"isOnChange: {IsOnChange}"); if (ElementAccessor.ToString() != "this") fields.Add($"elementAccessor: \"{ElementAccessor}\""); if (KoContext != null) fields.Add($"koContext: \"{KoContext}\""); if (CommandArgs != null) fields.Add($"commandArgs: \"{CommandArgs}\""); - if (!AllowPostbackHandlers) fields.Add("allowPostbackHandlers: false"); + if (AllowPostbackHandlers is {}) fields.Add($"allowPostbackHandlers: {AllowPostbackHandlers}"); if (AbortSignal != null) fields.Add($"abortSignal: \"{AbortSignal}\""); if (ParameterAssignment != null) fields.Add($"parameterAssignment: \"{ParameterAssignment}\""); return new StringBuilder("new PostbackScriptOptions(").Append(string.Join(", ", fields.ToArray())).Append(")").ToString(); diff --git a/src/Tests/Binding/JavascriptCompilationTests.cs b/src/Tests/Binding/JavascriptCompilationTests.cs index 68bda1aa66..c24797bf42 100644 --- a/src/Tests/Binding/JavascriptCompilationTests.cs +++ b/src/Tests/Binding/JavascriptCompilationTests.cs @@ -1637,5 +1637,4 @@ public enum TestEnum public TestEnum Enum { get; set; } public string String { get; set; } } - } diff --git a/src/Tests/Binding/KnockoutHelperTests.cs b/src/Tests/Binding/KnockoutHelperTests.cs new file mode 100644 index 0000000000..b8bb86c55b --- /dev/null +++ b/src/Tests/Binding/KnockoutHelperTests.cs @@ -0,0 +1,62 @@ +using DotVVM.Framework.Binding; +using DotVVM.Framework.Binding.Expressions; +using DotVVM.Framework.Compilation.ControlTree; +using DotVVM.Framework.Compilation.Javascript; +using DotVVM.Framework.Configuration; +using DotVVM.Framework.Controls; +using DotVVM.Framework.Testing; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.VisualStudio.TestTools.UnitTesting; + +namespace DotVVM.Framework.Tests.Binding; +[TestClass] +public class KnockoutHelperTests +{ + static readonly DotvvmConfiguration config = DotvvmTestHelper.DebugConfig; + static readonly BindingCompilationService bindingService = config.ServiceProvider.GetRequiredService(); + static readonly ICommandBinding command = bindingService.Cache.CreateCommand("_this.BoolMethod()", DataContextStack.Create(typeof(TestViewModel))); + static readonly IStaticCommandBinding clientOnlyStaticCommand = bindingService.Cache.CreateStaticCommand("_this.BoolProp = true", DataContextStack.Create(typeof(TestViewModel))); + + + [TestMethod] + public void NormalEvent_Command() + { + var result = KnockoutHelper.GenerateClientPostBackExpression("Click", command, new PlaceHolder(), new PostbackScriptOptions(abortSignal: CodeParameterAssignment.FromIdentifier("signal"))); + Assert.AreEqual("dotvvm.postBack(this,[],\"aS4YcJnv6U6PpCmC\",\"\",null,[\"validate-root\"],[],signal)", result); + } + + [TestMethod] + public void NormalEvent_StaticCommand() + { + var result = KnockoutHelper.GenerateClientPostBackExpression("Click", clientOnlyStaticCommand, new PlaceHolder(), new PostbackScriptOptions(abortSignal: CodeParameterAssignment.FromIdentifier("signal"))); + Assert.AreEqual("dotvvm.applyPostbackHandlers((options) => options.viewModel.BoolProp(true).BoolProp(),this,[],[],undefined,signal)", result); + } + + [TestMethod] + public void KnockoutExpression_Command() + { + var result = KnockoutHelper.GenerateClientPostBackExpression("Click", command, new PlaceHolder(), PostbackScriptOptions.KnockoutBinding with { AbortSignal = CodeParameterAssignment.FromIdentifier("signal") }); + Assert.AreEqual("dotvvm.postBack($element,[],\"aS4YcJnv6U6PpCmC\",\"\",$context,[\"validate-root\"],[],signal)", result); + } + + [TestMethod] + public void KnockoutExpression_StaticCommand() + { + var result = KnockoutHelper.GenerateClientPostBackExpression("Click", clientOnlyStaticCommand, new PlaceHolder(), new PostbackScriptOptions(abortSignal: CodeParameterAssignment.FromIdentifier("signal")).WithDefaults(PostbackScriptOptions.KnockoutBinding)); + Assert.AreEqual("dotvvm.applyPostbackHandlers((options) => options.viewModel.BoolProp(true).BoolProp(),$element,[],[],$context,signal)", result); + } + + [TestMethod] + public void Lambda_Command() + { + var result = KnockoutHelper.GenerateClientPostbackLambda("Click", command, new PlaceHolder(), new PostbackScriptOptions(abortSignal: CodeParameterAssignment.FromIdentifier("signal"))); + Assert.AreEqual("()=>(dotvvm.postBack($element,[],\"aS4YcJnv6U6PpCmC\",\"\",$context,[\"validate-root\"],[],signal))", result); + } + + [TestMethod] + public void Lambda_StaticCommand() + { + var result = KnockoutHelper.GenerateClientPostbackLambda("Click", clientOnlyStaticCommand, new PlaceHolder(), new PostbackScriptOptions(abortSignal: CodeParameterAssignment.FromIdentifier("signal"))); + Assert.AreEqual("(...args)=>(dotvvm.applyPostbackHandlers((options) => options.viewModel.BoolProp(true).BoolProp(),$element,[],args,$context,signal))", result); + } +}