-
Notifications
You must be signed in to change notification settings - Fork 97
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Binding system refactoring #341
Merged
Merged
Conversation
This file contains 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
…ree from parenthesis. * Added parenthesis resolving visitor, that inserts optimal braces to the expression. * View model annotations are used for smart observable handling, name of the property, etc. * Left artifacts of an experiment with javascript type-system annotations. It is not currently used.
…ion make it unnecesary
…command resolution, ...
…ow some test results.
* Removed DataContextSpaceId, repaced with the real DataContextStack with structural equality support * Added Internal.CurrentIndexBindingProperty property with prepared $index binding for easy use in Repeater and GridView
…bindings to reduce mess in the expression (from observables and null checks), fixed DataPager data contexts
…uld fix crashing Rewriting test
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.
So, finally, this is the refactoring, I'm promising you for about three months and will solve all the problems... It still needs some work, mainly on public API (that I promised to you) and of course bugfixing, but the system should be ready for review.
Motivation
see #297
TL;DR - bindings are too incapable, we need more features.
For example, one of the problem is, that when binding is compiled in one data context and then is transfered to nested one by some control or by property inheritance, the evaluation is broken and it throws strange errors or return undefined in JS.
Another problem is that sometimes control needs to touch the internal tree, which is impossible. It it even impossible to parse the binding to get the tree, because types of DataContext are inaccesible at runtime.
Features
Binding properties
Binding is now a key-value collection that contain all the stuff required to compile it, execute it on server, in JS, execute it in slightly different way, identify it on postback and so on. The point is, that everyone can define own properties and tell the compiler to instantiate them or compute them at runtime. Properties are implicitly computed when they are looked up by a set of resolvers from the properties that are already present.
Resolvers are registered in
BindingCompilationService
, at the binding itself byBindingCompilationOptionsAttribute
(applicable on binding class or on bound DotvvmProperty) or inBindingAdditionalResolvers
binding property (set by constructor with other properties). They are only executed only once per binding and binding instances are shared between all requests, so binding properties should be immutable, or at least thread safe, if you need to cache som state. It also means, that binding's public API is immutable, although internally properties are added as they are request.Resolvers can also be executed during compilation and the properties can be saved for runtime - it is configurable using
BindingCompilationRequirementsAttribute
which specifies which properties areRequired
(failure fails build) and which are onlyOptional
(failure is catched). To suppress inherited requirements from other places, it is possible to specifyExcluded
requirements. This attribute can be placed on binding class (e. g.ValueBindingExpression
), property that is bound or it can be one of the binding properties.To post-process property created by other resolver you don't have to create a dummy property and another for the real result - when resolver that returns the same property as it gets in arguments is registered, it will act as post-processor of the property, and can silently reassign it after creation - so you can for example have special Javascript transformation rules for specific property.
Improved Javascript compilation
JavascriptTranslator now translates binding to a syntax tree instead of a simple string, and then applies all kind of transformation to it, like ko.observable handling or adding null checks. And it is of course accessible as binding property, so you can apply your own transformations to it. The tree itslef would be a long story, if you are interested, feel free to ask and check out the implementation. For short - we use all kinds of trees and visitors in DotVVM, but while exploring NRefactory I found an interesing tree architecture that we haven't tried, so here it is :)
What has changed for everyone is that the produced JS is much nicer, does not contain unnecesary parens and ko.unwraps. And static commands return promise. Knockout binding expression can also be adjusted to correct data context, if the binding was moved to child control - see below.
DataContextStack in runtime
To solve the problem with binding execution in different data context that it was compiled, DotvvmProperty
Internal.DataContextTypeProperty
is added, that stores info about current data context to allow runtime checks and binding expression adjustment. If binding is applied in a context, that it was't compiled for, control's ancesors are enumerated and if the expected data context is found it is executed in context of this control. In Javascript expression, knockout context varialbes ($data, $context, ...) are prepended with$parentContext
identifier. In order to allow this transformation, currentDotvvmBindableObject
instance has to be known to theGetKnockoutBindingExpression
method and any other method that gets JS string from binding. Actually I left the old overload here and is it broken in the same way as it was before, so it's not techicaly a breaking change, but you should replace all usages.This feature actually means, that binding are now supported in inherited properties. It can be useful for view model injection to markup controls (cc @quigamdev)
Extension properties
The DataContextStack does not only contain the Type of viewModel,
ExtensionParameters
property was added. It can contain parameters that will be available in bindings in addition to _this, _parent, _root and ViewModel properties. They are set byDataContextChange
attribute, soCollectionElementDataContextChangeAttribute
now adds_index
parameter. They can be optionally inherited to nested data contexts, but parent context's extension parameters are currently inaccesible - if you have any opinion how it should work, feel free to share it in the comments.see #316
ApplyControlStyleAttribute
When control contains a static method marked with
ApplyControlStyleAttribute
it will be invoked at during view compilation, and you can modify the ResolvedControl here - add properties, child controls and so on. Just don't throw here errors, it would break the build, for control validation is thereControlUsageValidatorAttribute
.To do
There is still quite a lot of thighs to do.
BindingCompilationService
JavascriptTranslator
If you have any questions, ideas or feedback feel free to comment bellow.