Announcement

Collapse
No announcement yet.

Property initialization exception with latest Bridge.NET build

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

  • #16
    Thanks ProductiveRage

    1) To clarify: this limitation only would apply to Bridge.React components and not to any other class of our app which would not be a Bridge.React component, correct?
    2) Where would I then put the initialization logic for that property: I can't put it in the constructor neither since the Bridge.React does not allow that.

    Comment


    • #17
      Correct, it would apply to React components only.

      All of the configuration for a component should come from its Props class, are you sure that you really require any additional initialisation?

      Comment


      • #18
        >> All of the configuration for a component should come from its Props class, are you sure that you really require any additional initialisation?

        May be I don't understand ... but how would I initialize ChartCanvas.IsRenderRequired from the Props class?

        Comment


        • #19
          I can't answer that without seeing how IsRenderRequired is incorporated into the rest of the component - what sets it to true or false?

          "IsRenderRequired" sounds a bit like something that should be done via the React lifecycle method ShouldComponentUpdate (which looks at the current Props and State and the "next" Props and State and returns a boolean to indicate whether anything has changed that requires the component to change). I can't imagine why you would want to be doing rendering work within ComponentDidMount instead of in Render.

          If you want to set up initial values for properties or fields then you could do it in the GetInitialState lifecycle method but I still suspect that you're coming at things in a way that React isn't really designed for.

          Comment


          • #20
            ChartCanvas is a HTML5 canvas 'managed' by React. The visualization is 100% 2D custom rendering, React is not involved.

            ChartCanvas.IsRenderRequired is part of the view which is accessible to the model through a small C# interface, where the model would trigger (indirectly) a render pass by IsRenderRequired=true. The model has no clue about React (in fact we're using the exact same model for a Windows.Forms view) and would not know anything about Props and State of the view.

            Hence, my question on where to properly initialize .IsRenderRequired=true.

            Comment


            • #21
              Is this component one that you're writing yourself? You're just wrapping a canvas element and not a pre-made component that already does canvas work?

              If so, even when you maintain your own DOM elements within a component, you should still try to exist within the React lifecycle - for example, render the element(s) in the Render method.

              If you are then handling all of the interactions with your elements internally, then you probably need to override ShouldComponentUpdate and return false - otherwise, React will try to call Render and generate more DOM elements.

              For internal state, which is what I think that properties like "IsRenderRequired" are, it probably makes sense for you to put them into the State class and for you to override the GetInitialState method and configure an initial state reference there.

              I don't think that we have any public code that works with non-React-rendered components that I can share with you to demonstrate (though it should be fairly easy to find some JavaScript-based examples, most of the principles should be transferable to Bridge / C#). However, I have found some old code that we used to demonstrate how to wrap "CKEditor" in a React component. Note that this was for an old version of CKEditor (and so there are some hacks not required on recent versions and it relies upon the CKEditor JavaScript being loaded into the page already and I don't think it works with the current version of Bridge because there was a change that means that [External] on interfaces may have to be changed to [ExternalInterface].. the point of the following code is to try to illustrate roughly how you should do things, as opposed to offering you some ready-to-use code:

              using System;
              using Bridge;
              using Bridge.Html5;
              using Bridge.React;
              using ProductiveRage.Immutable;
              
              namespace BridgeReactCKEditorTest
              {
                public sealed class CkEditorTextArea : Component<CkEditorTextArea.Props, CkEditorTextArea.State>
                {
                  private IEditor _editor;
                  private bool _isMounted;
                  public CkEditorTextArea(string text, Action<string> onChange) : base(new Props(text, onChange)) { }
              
                  public override ReactElement Render()
                  {
                    return DOM.TextArea(
                      new TextAreaAttributes
                      {
                        ClassName = "editor",
                        Ref = textarea =>
                        {
                          // The "Ref" callback appears to get called by a "detachRefs" method within React during the
                          // "unmountComponent" process) - this call should be ignored since the editor reference will
                          // get tidied up in this class' ComponentWillUnmount implementation (and it doesn't seem like
                          // a good idea to rely upon this null-argument-Ref-call-during-unmounting since I haven't
                          // found official documentation about it)
                          if (textarea != null)
                          {
                            if (_editor != null)
                              _editor.Destroy();
                            _editor = CKEditor.Replace(textarea);
                            if (new Version(CKEditor.Version) < new Version(4, 2))
                            {
                              // The following events are required for CKEditor pre-4.2 (see
                              // https://www.drupal.org/node/2035615)..
                              _editor.On("key", CheckForContentChange);
                              _editor.On("paste", CheckForContentChange);
                              _editor.On("afterCommandExec", CheckForContentChange);
                            }
                            else
                            {
                              // .. more modern versions can just check "change"
                              _editor.On("change", CheckForContentChange);
                            }
                          }
                        },
                        // Set DefaultValue instead of Value because otherwise React will complain because we're not
                        // setting OnChange
                        DefaultValue = props.State.Text
                      }
                    );
                  }
              
                  protected override State GetInitialState()
                  {
                    return new State(text: props.Text);
                  }
              
                  private void CheckForContentChange()
                  {
                    // Use SetTimeout to ensure that any change to the editor has been reflected in the DOM (or, more
                    // accurately, to ensure that editor.getData() will return the new value - which it won't
                    // immediately, particularly for key press events(
                    Window.SetTimeout(
                      () =>
                      {
                        // Do nothing if the editor has already been destroyed between the event and this
                        // timeout-delayed event handler
                        if ((_editor == null) || !_isMounted)
                          return;
              
                        // If the new value is no different from the old one then do nothing (no point raising "Change"
                        // events if nothing's actually changed)
                        var newValue = _editor.GetData();
                        if (newValue == state.Text)
                          return;
              
                        // Update internal state and execute the "OnChange" callback to let the parent know that things
                        // are happening (we want the internal state to be consistent with the editor state at all
                        // times, which is why we call SetState here)
                        SetState(state.With(_ => _.Text, newValue));
                        props.OnChange(newValue);
                      }
                    );
                  }
              
                  protected override bool ShouldComponentUpdate(Props nextProps, State nextState)
                  {
                    // If the editor's already been destroyed then ignore this event
                    if (_editor == null)
                      return true;
              
                    // If this call appears to be due to a change in props then re-route it into a SetState call (which
                    // will result in this method being called again for the state change)
                    if (props.Text != nextProps.Text)
                    {
                      SetState(state.With(_ => _.Text, nextProps.Text));
                      return false;
                    }
              
                    // If this call is for a state change (which it must be if it's not for a props change, which was
                    // handled above) then ensure that the editor's content matches the new state. If this call was a
                    // result of the component's internal state being updated to match a change to the editor's
                    // content then the editor's content will (obviously) already have this new state value and no
                    // change is required. The only way that a change should be required to the editor's state is if
                    // this component has been re-rendered with new props (in this case, we don't really want to fully
                    // re-render the component - ie. unmount it and re-mount it - we only want to update the already-
                    // initialised editor's content). Note that any change to the editor's content will result in the
                    // user's focus being lost, so we want to avoid doing so where possible.
                    if (_editor.GetData() != nextState.Text)
                      _editor.SetData(nextState.Text, () => {});
                    return false;
                  }
              
                  protected override void ComponentDidMount()
                  {
                    // We need to track whether or not the component is mounted to ensure that we don't try to call
                    // SetState after it's been unmounted - it's possible that this could happen if one of the "On"
                    // event callbacks from the editor is fired after the editor is destroyed (if this callback results
                    // in a call to SetState)
                    _isMounted = true;
                  }
              
                  protected override void ComponentWillUnmount()
                  {
                    if (_editor != null)
                    {
                      _editor.Destroy();
                      _editor = null;
                    }
                    _isMounted = false;
                  }
              
                  // This property and the interfaces below act as bindings around a CKEDITOR reference that is expected
                  // to be already loaded and configured on the page
                  private static IEditorInitialiser CKEditor
                  {
                    get
                    {
                      // Note: This requires that the CKEDITOR reference be made available in JavaScript on the page
                      return Script.Write<IEditorInitialiser>("CKEDITOR");
                    }
                  }
              
                  [External]
                  private interface IEditorInitialiser
                  {
                    IEditor Replace(TextAreaElement container);
                    string Version { [Template("version")]get; }
                  }
              
                  [External]
                  private interface IEditor
                  {
                    void On(string eventName, Action callback);
                    string GetData();
                    void SetData(string value, Action complete);
                    void Focus();
                    void Destroy();
                  }
              
                  public sealed class Props : IAmImmutable
                  {
                    public Props(string text, Action<string> onChange)
                    {
                      this.CtorSet(_ => _.Text, text);
                      this.CtorSet(_ => _.OnChange, onChange);
                    }
                    public string Text { get; private set; }
                    public Action<string> OnChange { get; private set; }
                  }
              
                  public sealed class State : IAmImmutable
                  {
                    public State(string text)
                    {
                      this.CtorSet(_ => _.Text, text);
                    }
                    public string Text { get; private set; }
                  }
                }
              }
              I think that it should be acceptable to close this case now since it is veering off course into being a matter of how to integrate non-React components into React rather than being about a bug in Bridge.React.

              Comment


              • #22
                Thanks ProductiveRage

                I'll will check into our latest beta and amend our code as needed.

                Comment


                • #23
                  Moving thread to Help forum from Bugs.

                  Comment


                  • #24
                    ProductiveRage sorry I needed to revisit this issue...

                    As we moved forward it turns out that not being able to properly initialize properties of a class/component which is rendered by Bridge.React is a serious limitation.

                    Here is why:
                    - a component has 2 aspects: a rendering part and business logic
                    - I purposely don't want to have those 2 parts separated into 2 classes (like model and view), since it over complicates the design. Instead, I'm having a partial class which implements the (shared) business logic in one shared VS project and the rendering part as other piece of the partial class in a separate VS project which is technology dependent (Bridge.NET/Bridge.React, Windows.Forms, WPF, Xamarin etc.)
                    - the business part has tons of properties where it does not make sense to have them reflected in a state class, since they have nothing to do with rendering
                    - note: this thought process not only applies to the sample component above but any component which we ever will implement

                    This leaves the question: how and where could I properly initialize the properties of a Bridge.React component?

                    Comment


                    • #25
                      I'll say this bit first because I don't think that you'll like it and you may free to ignore it and move on.. :) I don't think that the design that you're proposing is a great idea. It sounds like you want to intentionally squash multiple responsibilities into single classes; business logic and rendering. This make each class longer and more complicated (and spread over multiple files since you're using partial classes) and it also makes the unit testing much more complicated - if your UI code is all in simple and "dumb" component classes then you can do all of the testing that relates to the DOM (or equivalent if producing code for other UIs) separate from the testing of the business logic. This is great because DOM testing is generally slow and requiring multiple browsers while testing of "pure" business code should be much cleaner and quicker - get some objects into a particular state, change something and check that the result is as expected. This separation is one of the things that I really like about React. It's also strongly recommended by the React Team that "presentation components" (those that directly render the UI) be "dumb" and generally:
                      • Have no dependencies on the rest of the app, such as Flux actions or stores.
                      • Don’t specify how the data is loaded or mutated.
                      • Receive data and callbacks exclusively via props.
                      • Rarely have their own state (when they do, it’s UI state rather than data).
                      • Are written as functional components
                      (Above list taken from medium.com/@dan_abramov/smart-and-dumb-components-7ca2f9a7c7d0).

                      Aside from this being good practice for code organisation (and particularly so for when working with React), you are making React's life more difficult when your components and not "Pure" - by which I mean that you're making React do more work which may one day result in poorer performance (hopefully it will never be perceptible but if it does become a problem one day then you'll find yourself in difficulty that will be awkward to get away from). A "Pure" component is one whose output only depends upon the Props value passed to it and not any other information. The Bridge.React library has a "PureComponent" class that automatically does some clever work when React thinks that it might have to re-render it and its children; it looks at the previous Props reference and what the new one would be and tells React if they're equivalent - if so, neither the component nor any of its children need to be considered by React for "consolidation" (where React updates its virtual DOM and then decides what changes need to be applied to the browser DOM). This mechanism means that most user interactions only force React to consider changes and perform consolidation for a small fraction of the virtual DOM, which makes life very easy for React. What I will show you below makes this impossible and so every interaction with the application will require React to perform reconciliation of the entire virtual DOM when, really, it doesn't need to. Poor React :(

                      So.. if you wish to ignore the above then there is an approach that springs to mind. With "stateful" components (those that are derived from the Component<TProps, TState> class, rather than PureComponent<TProps> or StatefulComponent<TProps>), the "GetInitialState" method may be overridden and this will be called early in the component life cycle - so you could perform field initialisation there. Something like this:

                      public class PropertyTestComponent : Component<PropertyTestComponent.Props, object>
                      {
                          private bool _initialised;
                          public PropertyTestComponent(Props props) : base(props) { }
                      
                          protected override object GetInitialState()
                          {
                              _initialised = true;
                              return null;
                          }
                      
                          public override ReactElement Render()
                          {
                              return DOM.Div($"Id: {props.Id}, Initialised: {_initialised}");
                          }
                      
                          public class Props
                          {
                              public int Id { get; set; }
                          }
                      }
                      If you think that this will be a common pattern then you might want to create a base class to hide away the "TState" details - eg.

                      public abstract class StatelessComponentWithInitialisation<TProps> : Component<TProps, object>
                      {
                          protected StatelessComponentWithInitialisation(TProps props) : base(props) { }
                      
                          protected sealed override object GetInitialState()
                          {
                              Initialise();
                              return null;
                          }
                      
                          protected abstract void Initialise();
                      }
                      Then you would write your component like this:

                      public class PropertyTestComponent : StatelessComponentWithInitialisation<PropertyTestComponent.Props>
                      {
                          private bool _initialised;
                          public PropertyTestComponent(Props props) : base(props) { }
                      
                          protected override void Initialise()
                          {
                              _initialised = true;
                          }
                      
                          public override ReactElement Render()
                          {
                              return DOM.Div($"Id: {props.Id}, Initialised: {_initialised}");
                          }
                      
                          public class Props
                          {
                              public int Id { get; set; }
                          }
                      }
                      Again, I very strongly recommend that you do not do this - I think that it's a difficult argument to make that separating business logic and rendering logic makes the design more complicated (it certainly won't making it any easier to test). If you really want to do this, though, then this is one way to manage it.

                      Comment


                      • #26
                        Yep, I'm well aware that my design approach is 'challengeable'. I'm trying different approaches right now and the one described above is the best one as it goes to code simplicity, readability, maintainability etc. I'm well aware of MVC, MVVM etc. ... but honestly they seem overly 'academic' and over-architected for what we're trying to achieve in our project.

                        That's how it looks right now ... may be my mind changes (again) as we progressed ... we'll see.

                        Thanks for your hint on the 'GetInitialState' method. We'll look into...

                        Comment

                        Working...
                        X