Add _page.Resource(...) – server-side evaluated part of value binding#2043
Open
exyi wants to merge 1 commit into
Open
Add _page.Resource(...) – server-side evaluated part of value binding#2043exyi wants to merge 1 commit into
exyi wants to merge 1 commit into
Conversation
tomasherceg
approved these changes
Jun 12, 2026
It is evaluated during initial request. Accesses to static properties now translate to this construct, so using resources should 'just work'
dc69a45 to
7e6ff23
Compare
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Adds _page.Resource(...) as a “server-evaluated” portion of value binding so that static properties / resources can be evaluated during initial render and then emitted as constants into the generated Knockout JS.
Changes:
- Introduces
_page.Resource<T>(T value)and translation support to evaluate subexpressions server-side. - Updates KO script formatting and argument substitution to support resource symbolic parameters that may require a target control.
- Adds extensive test coverage for resource evaluation semantics (data context shifting, serialization, and error cases).
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| src/Tests/Binding/JavascriptCompilationTests.cs | Adds tests covering _page.Resource(...) behavior plus static property/resource translation scenarios. |
| src/Framework/Framework/Controls/KnockoutHelper.cs | Hooks resource symbolic parameters into command argument substitution using the resolved data-context target control. |
| src/Framework/Framework/Compilation/Javascript/ParametrizedCode.cs | Adds a “try get cached default” helper used by formatting logic. |
| src/Framework/Framework/Compilation/Javascript/JavascriptTranslator.cs | Adds ResourceSymbolicParameter and updates KO formatting to optionally evaluate resources with a target control. |
| src/Framework/Framework/Compilation/Javascript/JavascriptTranslationVisitor.cs | Translates _page.Resource(...) and static properties into resource-evaluated symbolic parameters. |
| src/Framework/Framework/Compilation/Binding/GeneralBindingPropertyResolvers.cs | Passes binding instance into JS compilation for improved debug/error context. |
| src/Framework/Framework/Binding/BindingPageInfo.cs | Adds the new Resource<T> API with intended semantics. |
| src/Framework/Framework/Binding/BindingHelper.cs | Ensures KO formatting passes a target control for resource evaluation. |
| src/Directory.Build.props | Suppresses NU1900 warnings in builds. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
281
to
285
| p == CommandBindingExpression.CommandArgumentsParameter ? options.CommandArgs ?? default : | ||
| p == CommandBindingExpression.PostbackHandlersParameter ? CodeParameterAssignment.FromIdentifier(getHandlerScript()) : | ||
| p == CommandBindingExpression.AbortSignalParameter ? abortSignal : | ||
| p is JavascriptTranslator.ResourceSymbolicParameter resource ? resource.Binding(dataContextTarget.target) : | ||
| default |
Comment on lines
+194
to
+208
| public static string FormatKnockoutScript(ParametrizedCode expression, bool allowDataGlobal = true, int dataContextLevel = 0, DotvvmBindableObject? targetControl = null) | ||
| { | ||
| if (dataContextLevel == 0) | ||
| { | ||
| if (allowDataGlobal) | ||
| return expression.ToDefaultString(); | ||
| if (expression.TryToDefaultString() is {} defaultString) | ||
| { | ||
| if (allowDataGlobal) | ||
| return defaultString; | ||
| else | ||
| return expression.ToString(static o => | ||
| o == KnockoutViewModelParameter ? CodeParameterAssignment.FromIdentifier("$data") : | ||
| default); | ||
| } | ||
| else | ||
| return expression.ToString(static o => | ||
| o == KnockoutViewModelParameter ? CodeParameterAssignment.FromIdentifier("$data") : | ||
| default); | ||
|
|
||
| return resourceOnlyToString(expression, targetControl); |
Comment on lines
+478
to
+489
| public JsExpression TranslateResourceSubexpression(Expression expression) | ||
| { | ||
| InitOnlyPropertyCheckingVisitor.Instance.Visit(expression); | ||
|
|
||
| var replacementVisitor = new BindingCompiler.ParameterReplacementVisitor(DataContext); | ||
| var expr = replacementVisitor.Visit(expression); | ||
| expr = ExpressionNullPropagationVisitor.PropagateNulls(expr, _ => true); | ||
| expr = ExpressionUtils.ConvertToObject(expr); | ||
| expr = replacementVisitor.WrapExpression(expr, DebugBinding!); | ||
|
|
||
| var resultLambda = Expression.Lambda<BindingDelegate>(expr, BindingCompiler.CurrentControlParameter); | ||
| var resultDelegate = resultLambda.Compile(); |
| <LangVersion>13.0</LangVersion> | ||
| <!-- Disable warning for missing XML doc comments. --> | ||
| <NoWarn>$(NoWarn);CS1591;CS1573</NoWarn> | ||
| <NoWarn>$(NoWarn);CS1591;CS1573;NU1900</NoWarn> |
Comment on lines
1
to
6
| using DotVVM.Framework.Binding; | ||
| using DotVVM.Framework.Controls; | ||
| using DotVVM.Framework.Runtime; | ||
| using Microsoft.VisualStudio.TestTools.UnitTesting; | ||
| using System; | ||
| using System.Collections.Generic; |
Comment on lines
+115
to
+122
| internal string? TryToDefaultString() | ||
| { | ||
| // warning: this has weird semantics | ||
| // * in some cases, it will return null, even when default value could be initialized | ||
| // * then, after ToString is called, the ParametrizedCode notices all parameters are constants and sets the cache | ||
| // * subsequent calls will return | ||
| return this.evaluatedDefault; | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
It is evaluated during initial request.
Accesses to static properties now translate to this construct, so using resources should 'just work'