Giter VIP home page Giter VIP logo

Comments (9)

pngwn avatar pngwn commented on May 18, 2024

I've thought about this a little more and want to propose something, but I'd like some feedback so we can hopefully come up with something that makes sense here. I don't have the in-depth knowledge of the product that you do, so your input would be valuable.

There are a few issues I have noticed about the current state management system. In no particular order:

  • Sheer size: there are a large number of methods on the store that are responsible for different things. Navigating the store behaviour is confusing and time-consuming.
  • Multiple responsibilities: The store is responsible for updating state as well as making API requests to the server (mostly to persist state). It would be nice if this lives somewhere else. It is challenging to know which methods trigger side-effects and which don't at the minute.
  • Tight coupling: The view has direct dependencies on the store methods. Modifications to the store invariably require changes to the view as well. This isn't too bad right now (although not ideal), but could be more problematic in the future. Adding API requests when some state changes, for example, usually means inlining the API request in the store itself, which leads to more direct dependencies. This makes the sec more challenging. Maintaining what is a relatively complex piece of state management in this regard is a challenge: any changes in the store have to be mirrored in the view even for simple tasks, slowing down development.
  • No distinction between types of state: As I mentioned in the initial issue comment, there are different kinds of state, some of which is derived. Automatically deriving or computing state from other states will mean we have to manage less of the state manually. Getting hold of the current page object or the current list of screens, for example, are the kinds of assignments we should never need to make. Many of these can be automatically generated from that we do need to set. It would also be helpful to distinguish 'UI state' from 'app state'.
  • The view has access and control of everything: As it stands the view can modify pretty much anything it wants to. Aside from having access to every method on the store, which controls almost everything in it, the store is also writable, meaning the view can directly modify the state of the store if it wants to.

Schemas

I suggest we define some explicit schemas for pretty much all state and keep them well documented. There will be less back and forth when trying to keep UI and Server changes in sync, typescript could help here, but that is a discussion for another day.

We have some pretty set schemas right now. These are the schemas for pages, screens, and components (I'm abusing TS for the highlighting):

interface Style {
  [key: string]: string[];
}

interface Props {
  _component: string;
  _children: Props[]; // component definitions are just an object of props
  _id: string;
  _styles: {
    layout: Style;
    positions: Style;
  };
  _code: string;
  [key:string]: any; // this for additional arbitrary props
}

interface Page {
  title: string;
  favicon: string;
  stylesheets: string[];
  componentLibraries: string[];
  props: Props;
  _css: string;
  uiFunctions: string;
}

interface Screen {
  name: string;
  description: string;
  route: string;
  url: string;
  errors: [];
  props: Props;
}

We should document these somewhere other than here, is there an appropriate place where dev documentation could live?

I suggest we do the same thing for other data structures that are either suitably complex or are relied upon by multiple parts of the app. The current store looks like this:

interface Store {
  accessLevels: { levels: Array; version: number; };
  actions: Array;
  activeNav: string;
  appname: string; 
  apps: Array; 
  components: Array<Component>;
  currentComponentInfo: Component;
  currentComponentProps: ?; 
  currentFrontEndType: string;
  currentNode: ?; 
  currentNodeIsNew: boolean; 
  currentPageName: string; 
  currentPreviewItem: null; 
  errors: Array; 
  generatorLibraries: { [x:string]: Module }; 
  generators: Array<Component>; 
  hasAppPackage: boolean; 
  hierarchy: Hierarchy; 
  isBackend: boolean; 
  libraries: { [x:string]: Module }; 
  loadLibraryUrls: Function; 
  mainUi: {}; 
  pages: Array<Page>;
  screens: Array<Screen>;
  showSettings: boolean;
  triggers: []; 
  unauthenticatedUi: {};
  useAnalytics: boolean;
}

This is my first attempt at breaking these up into something more granular. The idea here is that the state will be easier to reason about as each state object is smaller and we will try to restrict state access only to those things that need to know about it. Not bulletproof, but it should offer a little more security. The UI and derived UI state might make more sense living on the same state object, I split it up here mainly because they update differently; we could merge them into a single object.

The state distinctions are as follows: App config, Frontend State/ Derived Frontend State, Backend State, Libraries.

I think this is all app-specific.

interface App {
  useAnalytics: boolean;
  name: string;
  pages: Array<Page>;
  components: Array<Component>; // definition only
  generators: Array<Component>; // definition only
  accessLevels: { levels: Array; version: number; };
  hierarchy: Hierarchy;
  triggers: [];
  actions: Array; 
  apps: Array; 
  errors: Array;
}

This is specific to the view that I know of:

type id = number | string // v4 uuids are strings

interface CurrentFrontendUi {
  type: 'page' | 'screen'; // it might be possible to derive this
  pageId: string; // maybe id, names may not be unique in the future
  screenId: id | null; 
  componentId: id;
  showSettings: boolean; // does this need to be global?
  hasAppPackage: boolean; // could this be inferred?
}

We can derive the following from the App and CurrentUi state:

interface DerivedUi {
  preview: Component; 
  ComponentInfo: Component; // props of current selection
  screens: Array<Screen>; // current screen definitions
}

I don't see any real reason why we need to store the actual library modules themselves with everything else; they can live alone and be used when needed:

interface Module {
 [x: string]: Function;
}

interface Library {
  [x:string]: Module;
}

interface Libraries {
  generators: Array<Library>;
  components: Array<Library>;
}

I know a lot less about the backend and what is required for the UI to work, but something like the following might make sense:

interface BackendUi {
  activeNav: ?; 
  currentNode: ?;
  currentNodeIsNew: boolean;
}

The loadLibrary function has gone, I don't know if that needs to be stored in here, it looks like a helper function. There were some pieces of state that don't seem to be globally required, I have removed some of them for now, but they could easily be readded.

I shall add more shortly with some suggestions regarding reducing coupling and separating responsibilities.

from budibase.

shogunpurple avatar shogunpurple commented on May 18, 2024

Love this idea. I've a suggestion about how we could go about implementing this and making some general improvements to our platform along the way.

1. Create .d.ts files for our schemas

As much as I think typescript would be the best thing for us to do (especially on the server side) we can migrate in a staged way by creating .d.ts files for defining the schemas above. This would give us an idea of what the state schema looks like and let us just copy and paste it when required. It would also be worth adding this schema to the README under the server package.

2. Move all state that can be derived out of the respective stores.

3. Identify where we can simplify the store data structures

We do a lot of client side logic to run finds over arrays in our stores. There are definitely gains we can make there - not only simplifying the code but reducing a lot of O(n)'s to O(1), which will make the apps (and builder experience) feel snappier.

4. Define a better way to update state

Coming from react world, I actually like the idea of having "reducers" to update these individual stores (functions that take a state update payload, and merge it into the main store state in an immutable way). I'm open to suggestions about this abstraction wise, but a nicer, type safe way of updating the builder (or client) state would be great. Right now, there are definitely hidden bugs we haven't identified that will write malformed JSON to screen files and app definitions, breaking the whole app and providing a generally nasty user experience. We should get ahead of this as the general layman user will have no idea how to resolve it

from budibase.

pngwn avatar pngwn commented on May 18, 2024
  1. Create .d.ts files for our schemas

I think TS would probably be a good idea in the long run. As an alternative, we can also use JSDOC comments, which will also kind of 'type' things for us. They would provide some type hints anyway and it is a very low friction way to getting started, they also allow us to add descriptions that will appear in IDEs, which isn't possible with TS alone. Linky: using JSDOC for types. Maybe we should do both, not sure.

  1. Move all state that can be derived out of the respective stores.

Strong agree. Although if the view needs access to the current screens and the current page name, for example, while those pieces of state will be updated in different ways, it probably makes sense for them to be available on the same object/ store from the view's perspective; they are just 'state'. Hopefully, the view won't be calling methods directly in the future, so the location of the state and the mechanism to update it could (maybe 'should') be separate. The view should have no knowledge of how things are updated, we should be free to change this at any time. It just needs to be able to access them in a simple manner.

  1. Identify where we can simplify the store data structures

We have a mix of arrays and objects right now. This is a tricky one because if we need to depend on the order (which we do in some cases) then arrays are a good choice, the order of keys on an object is generally standard but can't be relied upon. But arrays do make lookups more challenging, storing references to indices isn't reliable since we would be relying on the index for ordering purposes as opposed to identity. If we track by ID (or object identity) then we are back to array.find. In general, we can probably switch to objects for all collections that don't need a specific order, if we key by ID then we should be able to reduce most finds to O(1) lookups. That said will the user be able to order stuff in the future? Pages and Screens as well as components? If so then everything needs to be an array. Alternatively, we can track all orders separately using an array of IDs, keeping all components and pages as objects keyed by ID.

All of this said, these aren't large data structures and this may just be an overcomplication. Performing O(n) operations on collections smaller than a few hundred items should be incredibly fast. If we do have some performance issues, we may want to look more closely at what is happening inside those operations. In all likelihood, the preview rendering is the big bottleneck and this would probably block the main thread completely until it was done. Would need to do some profiling to confirm.

  1. Define a better way to update state

I have more on this but for now I'll say that redux is basically just pubsub in disguise and I don't love the reducer pattern as it is in redux: I think there are simpler ways to update state without giving up the benefits of immutability.

from budibase.

pngwn avatar pngwn commented on May 18, 2024

Regarding state updates.

There are a few options here, the important thing is to completely decouple the view from the store, moving away from direct method calls to dispatching events (or actions, whatever you want to call them).

There difference is pretty significant:

store.someMethod(payload);
store.someOtherMethod(payload);
store.someDifferentOtherMethod(payload);

vs:

store.send('action', payload)
store.send('action-two', payload)
store.send('action-three', payload)

Here we just send off a random event to a single clearly defined interface (it is just an event bus really), what the store does with that event and payload is entirely up to it. If we ever want to change the internal implementation or stop responding to an event, then we are free to do so. Inside the bus we can simply choose to respond to events as needed. The two components are completely separate, neither having knowledge of the otehr. As long as the stores stay consistent with the schemas we design, we are depending on a generic interface rather than having any direct dependencies.

This also gives us the opportunity to have multiple subscribers for events:

store.on('action', payload => updateState(payload))
store.on('action', payload => apiRequest(payload))

In this way we can completely split out the API requests and the State updates, this could also allow us to cleanly perform optimistic UI updates, changing the UI state and reverting it if any associated API requests were to fail. Keeping the UI 'eventually consistent' with the DB (this isn't the standard eventual consistency but a similar concept).

We can probably do better than the above API, with on handlers having direct access to state. You'll notice that this is conceptually similar to redux but more flexible out of the box, without excessive boilerplate to combine reducers. You can call on on a store at any point to add a 'state handler. The state can be safely mutated (the state argument would be a deep clone), we don't need proxies for this, which come with a significant performance cost.

export const store = createStore(initial)

store.on('event', (state, payload) => {
  state.push(payload)
})

What we do need to think about is having a single interface for events but the ability to update a number of different stores. The views themselves will import a readonly store that is updated via these events. This could be a global bus that delegates events to child event-based stores?

Something like this:

+--------------------------------------------------+
|                        View                      |
+------+--------+---------+--------+--------+------+
       |        |         |        |        |
       |        |         |        |        |
       |        |         |        |        |
       v        v         v        v        v
+------+--------+---------+--------+--------+------+
|                     GLOBAL BUS                   |
+------+------------------+-----------------+------+
       |                  |                 |
       |                  |                 |
       |                  |                 |
       v                  v                 v
+------+-------+  +-------+------+  +-------+------+
|     App      |  |      UI      |  |    Backend   |
+--------------+  +--------------+  +--------------+

Each of these mini, end stores would have a subscribe method (so svelte components can still auto subscribe/unsubscribe) but they would be read-only, it wouldn't be possible to write to them directly.

Other parts of our app could also subscribe to them (like to send API requests) and would also receive the payload, and maybe the state?

This is a bit rough, but I figured I'd get something out even if I haven't fully thought it out yet. I have used systems similar to this with some success in the past. We get clean separation, a good interface to split up responsibilities, immutable state updates, and easy state access for the view. Both event names and the state objects could be typed using TS as well, so we could get strict compile-time checks to provide further safety.

The flipside to all of this is that it introduces more complexity that we will need to manage. I personally think the trade-off is worth but others may disagree. Would love to hear what people think!

from budibase.

shogunpurple avatar shogunpurple commented on May 18, 2024

Very nice - that's the idiomatic svelte version of what I was thinking in react (actions/payloads in a pub sub manner to represent state updates and separate concerns between view and store). This design also allows us to have many more "hook" points in budibase as we can continue to add more subscribers.

It would appear that we could actually implement this store quite easily with a small wrapper around a writable store.

If I'm understanding correctly, it looks like the best way to approach this would be:

  • Move any store. methods into the global bus, import them and update where they are called from the views
  • Create our createStore wrapper around writable that knows about payloads and actions
  • Update the store. methods above to: store.send('action', payload) which will update the state in the new action-aware store.
  • Replace subscriptions in the views to the old store to use the new action-aware one
  • Start to split out the big, main store into different stores (App, builder, derived etc)
  • Update views as appropriate to use the relevant store

from budibase.

mjashanks avatar mjashanks commented on May 18, 2024

I pretty much agree with everything in here, and see the benefits.

I will continue to give feedback if required, but am happy to leave it in your capable hands to decide how and when is the right time to implement... here are some questions that I would like you to consider when making the decisions

What value will this bring

  • will this make the code more or less understandable, without having to read this issue
  • More maintainable (i think "yes" is the main point of the change)
  • Performance affected - positively/negatively/negligibly
  • Testability (really want to add test coverage to the stores)

Is now the right time?

  • how long will this take, including design time & unit testing of the message bus layer (i.e. not thinking about testing the store message handlers at this point)
  • will we all have to find other things to do while it's implemented ? If so, can more than one person work on it?

... i.e. if it takes 2 days, and nobody can work on the builder for that time, then it sets us back 6 person days (we have a demo to produce at the end of Feb)

from budibase.

mjashanks avatar mjashanks commented on May 18, 2024

P.S this is overkill probably.... but...

Since this is all message passing, is there any worth in running this entire layer in a worker ? Seems to lend itself to it.

Probably overkill.

from budibase.

pngwn avatar pngwn commented on May 18, 2024

will this make the code more or less understandable, without having to read this issue

Should be better, there will be a cleaner separation and the code will be more modular with each module only concerning itself with one thing. This should make things easier to navigate. Some documentation will still be necessary for a complete understanding but in general, I think it would be an improvement.

More maintainable (i think "yes" is the main point of the change)

Yeah, the improved modularity would make it easier to modify and add features, the cleaner separation should mean that changes in the store/view/api don't affect anything else.

Performance affected - positively/negatively/negligibly

There shouldn't be any noticeable difference assuming we update state in the same way.

Testability (really want to add test coverage to the stores)

This should improve the testing situation, since we would just be relying on interfaces, we can test this entire layer (and possibly any 'sublayers') in complete isolation manually dispatching events. It will also make mocking easier by separating state updates from API calls.

from budibase.

pngwn avatar pngwn commented on May 18, 2024

.There was some further discussion last night of extending this change to use state-charts (hierarchical state machines) to control state as well. This would still have most of the benefits discussed above; it is still an event-based system with a generic interface giving the clarity and separation we desire. The implementation, however, is significantly different, and it also has ramifications for the rest of the app.

To adopt a state-machine driven architecture, we would need to model the abstract state of our app. This applies only to 'finite' (known, countable) state. The process is essentially one of listing the possible 'states' your app (or a part of it) and then defining legal transitions. A legal transition is a transition between two states. When working with state machines, everything is explicit; if you don't allow one state to move into another, then it cannot happen. This offers a level of safety that is otherwise difficult to achieve.

Here is a quick sketch of a basic machine. The blue boxes are states, and the orange boxes are events which trigger transitions (move the machine to a new state). The green mark is the initial state, and the red mark is a final state. State machines can have a 'final' state which signals the completion of that machine; this also lets any parent machines know that it is 'done'.

machine

If there is no arrow, then there is no transition. This machine, for example, can never move from idle directly to error or success. Likewise, once it has left the initial state, it can never go back to idle. Without the explicit transitions, this machine could have a dozen or so transitions; our constrained version has 4. This is often a source of bugs, conflicting states, or transitioning into a state that requires some data that isn't yet available. With a constrained system like this, we explicitly allow state transitions, rather than selectively disallowing them when undesirable behaviour occurs. The latter approach is unsustainable when building complex UIs.

Additionally, we can attach actions to states or transitions. This triggers some 'effect', updating state or an API call etc. Certain types of action/effect can be directly modelled as state machines, such as promises.

Data is considered 'extended state', you would update this data using actions as well. The machine controls everything.

The main benefit of this architecture is that we have a safe, declarative method with which to model the state of the application. All behaviour is strictly constrained by this system, making awkward bugs from conflicting states less likely, state. Updates and side-effects are attached to states or state transitions so we can associate 'behaviours' with individual states too.

Additional benefits include auto-generated documentation, auto-generated tests, a clear language with which to discuss app state. One interesting feature is that this approach can help bridge the gap between product and dev. At a high-level, statecharts can be understood by anyone; it is relatively straightforward to convert a UX flow diagram into a state machine with a little practice. In my opinion, bringing product and development into closer alignment is incredibly valuable. State-machines can be part of a common language that we can use to discuss app logic across the team as they are pure logic, no code. A better aligned and better-informed team will lead to less friction between business, product and engineering in the long term.

The main challenge we face when implementing a system like this is mainly due to familiarity. Modelling app state in this way is an entirely different approach to more traditional methods (on the frontend at least), it takes some getting used to. This cost is mostly upfront, and I expect the onboarding cost to reduce over time as state machines become more common on the frontend, but we aren't there yet. Additionally, designing a good model does take some time, after you have the model things are generally straightforward, but there can sometimes be some back and forth. New models should always be discussed and reviewed, these are rigid structures, and the model needs to reflect the desired behaviour accurately. A mistake in the design of the model isn't catastrophic (it can be changed), but it is generally a good idea to get a few eyes on them to make sure there are no glaring mistakes or limitations that may cause problems. This rigidity can also be a potential problem, but as long as the models are correct, this shouldn't be much of an issue Models are expected to change over time as the app evolves, these modifications are not an issue.

Right now, the most significant cost to us is going to be modelling the app. This will take a little time. It is difficult to incrementally adopt state machines in this way because they want to control everything. This makes extended state problematic: to incrementally adopt, we would need to be updating state from multiple places. We could pass the existing store into the machine until we are ready to transition fully. I haven't had the chance to fully consider how we would implement this and how we would handle state in this context (in relation to some of the discussion above), but I think those challenges can be overcome.

The most popular statechart library for front end work is xstate, which is mature and well-documented, even if it is a little large. There are a few other options, but we'd be pretty much on our own then.

I want to point out that although I am a big proponent of state machines for frontend work, having used them successfully in the past, this is a significant change and will have a significant impact on how we build the front end.

Let the discussions commence. Again.

from budibase.

Related Issues (20)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.