Uploaded image for project: 'Jenkins'
  1. Jenkins
  2. JENKINS-36439

Evolve more composable redux store code patterns

    Details

    • Similar Issues:
    • Epic Link:
    • Sprint:
      1.0-m11, 1.0-m12

      Description

      This is assigned to Tom FENNELLY for now, but it's really Thorsten Scherler + Tom FENNELLY, working together to evolve the current redux codebase to something that is easier to maintain, test, extend etc.

      We are using redux more and more and it works well (it's fundamental to how BlueO works), but we have stretched the current implementation beyond the point where it is easy/possible for others to make sense of it, contribute to it etc. It's getting harder and harder to maintain, so we really need to evolve it to the next stage.

      There may be other evolutionary steps required/possible (e.g. generating from the backend rest api) as the whole app evolves.

      We'll see if we can come up with a more composable pattern for managing app state in redux. Maybe something like a StoreObject JS type that can be added as an extension point (in the .yaml file) that encapsulates a redux reducer for handling a specific set of redux actions relating to a specific piece of state (e.g. branches on a job) + providing helpers for loading/fetching data from the REST API etc.

      Let's see about Immutablejs too. Thorsten Scherler feels strongly that this is the right way to store the state. He has very valid points re reference Vs value equality etc and how Immutablejs helps there + management of large amounts of data (freezing and cloning the whole state will not scale as we grow), but others find Immutablejs a bit cumbersome to use (myself included). Maybe there's a happy medium..

      Moving everything to a new pattern in one go would be a big task, so it would be good if the new pattern could live alongside the existing stuff as we transition. That way, we can divide and conquer.

        Attachments

          Issue Links

            Activity

            Hide
            tfennelly Tom FENNELLY added a comment -

            I know Thorsten Scherler mentioned perhaps ditching connect and just passing props down the entire view hierarchy - although I'm not sure if there's an automated way to do this, or if it has to be manual at each level. Connect allows us to avoid writing container components and a lot of boilerplate, but it's so terse that it's extremely hard to follow and debug. Maybe I just need more practice with it.

            Yeah, I'm not at all a fan of connect. It removes so much that it's more or less impossible grok what's happening without having a PhD in redux. That's not good at all from a code maintenance pov.

            Connect is really just a wrapper around passing the store via the react context from a top level <Provider>. You can do that easily enough by hand. It also handles the component store subscribe/unsubscribe to trigger the render/rerender, which is handy but can also be done easily enough by hand.

            There are other things like this in some of these libs that I do not like at all. Some people seem to have the idea that the only thing that matters is the terseness of the code, which is a huge anti-pattern imo. I'm much more in favour of a bit more code that is easier to follow and a bit more self explanatory i.e. not requiring the PhD in redux before you can make sense of it.

            Show
            tfennelly Tom FENNELLY added a comment - I know Thorsten Scherler mentioned perhaps ditching connect and just passing props down the entire view hierarchy - although I'm not sure if there's an automated way to do this, or if it has to be manual at each level. Connect allows us to avoid writing container components and a lot of boilerplate, but it's so terse that it's extremely hard to follow and debug. Maybe I just need more practice with it. Yeah, I'm not at all a fan of connect. It removes so much that it's more or less impossible grok what's happening without having a PhD in redux. That's not good at all from a code maintenance pov. Connect is really just a wrapper around passing the store via the react context from a top level <Provider> . You can do that easily enough by hand. It also handles the component store subscribe/unsubscribe to trigger the render/rerender, which is handy but can also be done easily enough by hand. There are other things like this in some of these libs that I do not like at all. Some people seem to have the idea that the only thing that matters is the terseness of the code, which is a huge anti-pattern imo. I'm much more in favour of a bit more code that is easier to follow and a bit more self explanatory i.e. not requiring the PhD in redux before you can make sense of it.
            Hide
            cliffmeyers Cliff Meyers added a comment -

            I agree. We need to keep the learning curve shallow so plugin authors can make sense of this stuff. We can chat more about it tomorrow.

            Show
            cliffmeyers Cliff Meyers added a comment - I agree. We need to keep the learning curve shallow so plugin authors can make sense of this stuff. We can chat more about it tomorrow.
            Hide
            cliffmeyers Cliff Meyers added a comment -

            I should add, it would be great if we could add some logic to the "generateData" / related utils to immediately transform the object literals translated from JSON into instances of model classes / objects. That way we have some type consistency throughout the app, rather than various components handling the translation on their own. I think whether we use Immutable.js is a separate issue, really just an implementation detail.

            Show
            cliffmeyers Cliff Meyers added a comment - I should add, it would be great if we could add some logic to the "generateData" / related utils to immediately transform the object literals translated from JSON into instances of model classes / objects. That way we have some type consistency throughout the app, rather than various components handling the translation on their own. I think whether we use Immutable.js is a separate issue, really just an implementation detail.
            Hide
            tfennelly Tom FENNELLY added a comment -

            Update on what we did: https://github.com/tfennelly/blueocean-plugin/blob/JENKINS-36439/redux/README.md

            Work on this has been parked for now.

            Show
            tfennelly Tom FENNELLY added a comment - Update on what we did: https://github.com/tfennelly/blueocean-plugin/blob/JENKINS-36439/redux/README.md Work on this has been parked for now.
            Hide
            tfennelly Tom FENNELLY added a comment -

            Closing this as Cliff is already working on introducing MobX in the personalization.

            Show
            tfennelly Tom FENNELLY added a comment - Closing this as Cliff is already working on introducing MobX in the personalization.

              People

              • Assignee:
                tfennelly Tom FENNELLY
                Reporter:
                tfennelly Tom FENNELLY
              • Votes:
                0 Vote for this issue
                Watchers:
                3 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved: