-
Story
-
Resolution: Won't Do
-
Major
-
-
1.0-m11, 1.0-m12
This is assigned to tfennelly for now, but it's really tscherler + tfennelly, 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. tscherler 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.
- is blocking
-
-
- Closed
-
[JENKINS-36439] Evolve more composable redux store code patterns
Right, any feature that is not storing state in the redux store doesn't care about this.
Any feature that does store state is not hard blocked by this i.e. we could just keep going as-is, but imo we should not put off trying to clean up the redux code any longer as it's getting more and more unmaintainable with every new feature contributing to it.
tfennelly understood, and it makes sense to make pagination depend on this stuff? (as in, not happen at the same time)?
michaelneale as I said, we could forge ahead with pagination and all other features without doing this, but that's a bad idea imo as it's just putting off the inevitable and making it a bigger job when we do it (more code to refactor).
I know tscherler 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.
I'm setting up a new store for Personalization and trying to tweak the organization of things a little bit, so we should keep in touch about where you guys settle. The ticket FYI is JENKINS-35837.
I would also like to add that we should put a little thought into what we might be able to code generate later - particularly ImmutableJS classes that match against the objects sent over the wire in JSON. Don't need to write the code to do that yet, but maybe we want to structure things in such a way that each class is in its own file, and that are all imported/exported via a single manifest file. e.g.
model/
– index.js (exports all classes)
– Run.js
– Pipeline.js
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.
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.
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.
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.
Closing this as Cliff is already working on introducing MobX in the personalization.
I think this is only a blocker in the sense that it is "very important" for immediate things that are desperately needed like paging, it isn't blocking most other features that are currently being worked on.