Skip to content
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

Version 2.1.2 breaks auto-property constructor initialization #265

Closed
cervengoc opened this issue Jun 27, 2017 · 10 comments
Closed

Version 2.1.2 breaks auto-property constructor initialization #265

cervengoc opened this issue Jun 27, 2017 · 10 comments
Milestone

Comments

@cervengoc
Copy link

Hi, when I updated to version 2.1.2 our entire application broke because as it turned out, during the IL weaving the auto-property initialization in the constructor have been replaced by direct backing-filed assignments.Here is a short example code.

public class Model {
  public int Something { get; set; }

  public Model() {
    Something = 1;
  }
}

In the generated assembly the Something = 1 property assignment is turned into assigning the value 1 to the corresponding backing field.

Reverting to 1.53.0 the problem is gone.

Is this behavior by design? If so, I couldn't find any notes anywhere about such a breaking change.

@navozenko
Copy link

As far as I understand, this was done in version 2.1.2 to exclude the invocation of virtual methods from the constructor.
#253, #220

@distantcam
Copy link
Member

distantcam commented Jun 28, 2017 via email

@navozenko
Copy link

navozenko commented Jun 28, 2017

@distantcam inside a class, there may be owned handlers that are necessary to ensure the consistent internal state of the instance. For example, the handlers can subscribe to events or recalculate the values of other properties. If the constructor does not call handlers, then the created instance may be in an inconsistent state.

It seems to me that version 2.1.2 is a mistakes that breaks the work of a large number of programs. In addition, the new behavior is confusing, unexpected and unobvious.

For example, next class don't work in v2.1.2:

[AddINotifyPropertyChangedInterface]
class Model
{
    public Model()
    {
        Foo = 1;
    }

    public int Foo { get; set; }
    public int FooPlus1000 { get; private set; }

    void OnFooChanged()
    {
        FooPlus1000 = Foo + 1000;
    }
}

@cervengoc
Copy link
Author

@navozenko I totally agree, I think the idea is basically good, but this should definitely be some kind of explicitly opt-in feature if developers want to make use of it.

By the way, my scenario was exactly a kind of internal state management affecting lots of our view model classes` initial state.

@tom-englert
Copy link
Member

#264 will now have the old behavior as default; suppressing property changes from within the constructor can be controlled by the new NotifyAutoPropertiesInConstructorAttribute

@Syn-McJ
Copy link

Syn-McJ commented Jun 30, 2017

I guess this is also why x:Bind is broken: if you initialize your properties from constructor, x:Bind simply will not update UI anymore if it set one-way or two-way.

@SimonCropp
Copy link
Member

ok for now i am reverting #253 until i have more time to revisit this

@SimonCropp SimonCropp added this to the 2.1.3 milestone Jun 30, 2017
@SimonCropp
Copy link
Member

pushed to nuget

@tom-englert
Copy link
Member

@SimonCropp then please re-open #220.

@SimonCropp
Copy link
Member

Done

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants