Announcement

Collapse
No announcement yet.

[CLOSED] [#377] [1.9] ObjectLiteral classes generate values for all properties in 1.8 (changed from 1.7)

Collapse
X
  • Filter
  • Time
  • Show
Clear All
new posts

    [CLOSED] [#377] [1.9] ObjectLiteral classes generate values for all properties in 1.8 (changed from 1.7)

    I'm not sure if you would consider this a bug or not, but I think it's a change from 1.7 to 1.8 and the old behaviour was better for my particular use case! Whether you agree that it would be better overall might be a different matter :)

    I'm trying to write some React bindings for Bridge and I'm using the [ObjectLiteral] attribute on classes that declare properties for elements - eg.

    // Class cut right down for illustrative purposes
    [ObjectLiteral]
    public class TableCellAttributes
    {
        public string id;
        public string className;
        public int colSpan;
    }
    When I use this to create a new table cell - like

    React.DOM.td(new TableCellAttributes { className = "test" })
    Before, this would generate code such as

    React.DOM.td({ className: "test" })
    but now the translated code sets a value for every property - eg.

    React.DOM.td({ className: "test", id: null, colSpan: 0 })
    This doesn't quite have the same meaning. I'm not 100% sure what setting a "colSpan" of zero would even do! If I change "colSpan" to be a nullable int then the translated code becomes

    React.DOM.td({ className: "test", id: null, colSpan: new Bridge.Int() })
    and I definitely don't know what that will do! I have no clue if React will interpret that "Bridge.Int" value happily or not.

    This is why the old way (which is how I observed it when I was compiling with Bridge 1.7, I think that it's only changed in 1.8 but I couldn't see anything in the release notes / fixed-issues / new-features / unit-tests about it, so I'm not absolutely sure that it's a 1.8 thing or even an intentional thing) is better.. for this case. It was really good to know that only the properties I was explicitly setting would make it through to the translated code, in the resulting object literal, and the properties that I didn't set would not appear. Another benefit is that the translated code would be shorter, since it would not have to include properties that did not explicitly get set in the C# code.

    However.. I can sort of imagine an argument why the 1.8 approach might be considered superior; because it ensures that properties which are of type "int" do in fact get an int value (otherwise they would be left undefined).

    Can I do that annoying thing where people with problems suggest solutions, even when they are not necessarily in possession of all of the facts and consideration gone into a design decision? :D Obviously, I would love for the behaviour to revert back, but it would be just as good for this to be an option in some way. Something like an optional flag on the [ObjectLiteral] attribute, such as

    [ObjectLiteral(IncludeUnspecifiedProperties = false)]
    .. or even a new [Optional] attribute that could be used on the individual properties.

    My point is that I'm not too bothered if you think that the behaviour is 1.8 is more correct and you don't want to revert it by default, but I do think it would be good to have some mechanism to opt into the old way.

    Thanks for reading, I hope that all makes sense!
    Dan.
    Last edited by Daniil; 2015-08-20 @ 06:10 PM.

    #2
    I've had a bit of a breakthrough!

    If I use [Ignore] and [ObjectLiteral] then I get the behaviour I want - eg.

    [Ignore]
    [ObjectLiteral]
    public class TableCellAttributes
    {
        public string id;
        public string className;
        public int colSpan;
    }
    will result in a C# statement such as:

    Console.WriteLine(new TableCellAttributes { className = "test" });
    being translated into JavaScript thusly:

    console.log({ className: "test" });
    but if I don't include the [Ignore] attribute -

    [ObjectLiteral]
    public class TableCellAttributes
    {
        public string id;
        public string className;
        public int colSpan;
    }
    then

    Console.WriteLine(new TableCellAttributes { className = "test" });
    becomes

    console.log({ className: "test", id: null, colSpan: new Bridge.Int() });
    So, I think my question now becomes: Am I doing this correctly or am I relying upon undocumented behaviour?

    PS: I was trying to demonstrate this on the "Live" page, but in Chrome I couldn't reproduce it any more (it was acting as if [Ignore] was always present - ie. the unspecified properties were no longer being included) and in Firefox and IE I was getting an error like:

    // Line 0, Col 0 : Could not find file '\\fs6-n02\stor14wc1ord1\956235\live.bridge.net\web\conte nt\Bridge\Builder\pukam5dkwzetw3ec2bitjukv.dll'.

    Comment


      #3
      Hi ProductiveRage,

      I'm not entirely sure why these default values are being rendered in 1.8.0. I wasn't aware of change to this logic, but I will discuss with the team.

      Comment


        #4
        Hi ProductiveRage,

        It appears this change in functionality was the result of another problem that we were attempting to solve. Looks like it had some unintended consequences.

        We are reverting back the default value functionality, and this fix will be available in new patch release to be published soon.

        Comment


          #5
          Created an Issue:
          https://github.com/bridgedotnet/Bridge/issues/377

          I agree the previous behavior should be reverted back. Having a class/filled attribute to optionally emit default values would be good to have as well.

          I will create a unit test for that issue a bit later.
          Last edited by Daniil; 2015-08-20 @ 06:29 PM.

          Comment


            #6
            Excellent news! Thanks for getting back to me so quickly (and with the answer I wanted!).

            Comment


              #7
              The fix has been committed to the report. I will close the thread once I create a unit test.
              Last edited by Daniil; 2015-08-24 @ 11:23 AM.

              Comment


                #8
                A unit test has been added. It is already auto-deployed online.
                http://testing.bridge.net

                Comment


                  #9
                  Hi ProductiveRage ,

                  We have implemented feature Ability to emit default values for ObjectLiteral.
                  Now you have control on how default values are emitted. It will be included in 1.9 release.

                  Comment

                  Working...
                  X