search

elastic / kibana

Your window into the Elastic Stack
https://www.elastic.co/products/kibana
Other
19.65k stars 8.23k forks source link

Create Redux Style Guide #11389

Closed weltenwort closed 5 years ago

weltenwort commented 7 years ago

:construction: WIP

Proposed Conventions

Most of these proposed conventions have been described in detail in https://github.com/erikras/ducks-modular-redux and https://jaysoo.ca/2016/02/28/organizing-redux-application/.

Group Files by Feature/Domain

Consume Modules Only via Their Public Interface

i.e. do not bypass the index.js to import from deep inside a module.

Define Clear Public Interfaces for Modules

export * as actions from ‘./actions’;
export * as constants from ‘./constants’;
export { myModuleReducer as reducer } from ‘./reducer’;
export * as selectors from ‘./selectors’;

Usually, the reducer is exported as the default export of a module, but in accordance with our no-default-export policy it can just as well be named reducer.

Handle Side-Effects In Redux Middleware

e.g. a SearchSourceMiddleware triggered via EFFECT_SEARCH_SOURCE actions

Delegate From Global Selectors to Local Selectors

Since they both encapsulate the knowledge about the shape of the same part of the state tree, they should be co-located in the reducer.js and selectors.js of the same module. To avoid breaches of the module boundaries in spite of the reducer/selector asymmetry, selectors located higher in the module hierarchy delegate to the local selectors (as demonstrated by Dan Abramov in https://egghead.io/lessons/javascript-redux-colocating-selectors-with-reducers).

kimjoar commented 7 years ago

As far as I can see none of those resources you link discuss the side-effects handling. Can you elaborate on that? Would be great with blog posts or other sources that elaborate on that style, and preferable a resource that compares it to relying on e.g. redux-thunk and performing side-effects in the action creators.

weltenwort commented 7 years ago

@kjbekkelund absolutely! I haven't gotten around to fleshing out that section yet, so let my try to do so now:

The best summary of that aspect I have found is http://blog.isquaredsoftware.com/2017/01/idiomatic-redux-thoughts-on-thunks-sagas-abstraction-and-reusability/. While I mostly do not agree with the conclusions the author draws, it shines a pretty good light on various aspects of the fat-thunk vs middleware discussion.

The biggest mistake the author makes is to put thunks and sagas on the same side of the argumentation. IMHO, sagas are a way to writing custom middlewares in a testable way while thunks are clearly located on the "put more logic into the action creators"-side of things. I was considering recommending the usage of either redux-saga or redux-observable (if we want to stick with observables instead of generators) in Kibana, but left it out because I wanted to avoid another concept during this introductory phase. The upgrade path from custom middleware to redux-observable is quite seamless though, so I don't see a problem with that delay.

I want to make it clear that I'm not arguing against the necessity of flow control in action creators, but against directly embedding side-effectish code into them (see below for some reasons). On the contrary, I think we might be well advised to use something like redux-effects to have powerful, declarative means of composing action creators while keeping them testable.

That being said, from experience collected in prior projects and from trying to integrate redux and angular during the past days, these are my thoughts and observations regarding the use of middlewares:

Some examples from my current WIP context view branch to round things off:

They demonstrate how easy it is to inject dependencies from the angular world at a few places only. If these ever get refactored for use without angular, only a handful of middlewares instead of a huge amount of action creators need to change the way they are imported.

The notify middleware also shows how convenient it is to implement cross-cutting concerns that are very loosely coupled with the rest of the code.

I hope I am making some amount of sense here and look forward to your feedback. :sweat_smile:

cjcenizal commented 7 years ago

Thanks for getting this started Felix! I have a couple thoughts.

Group Files by Feature/Domain

I'm cool with experimenting with combining the reducer and action creator layers, but I'm concerned about combining these layers with the view/component/UI layer. I'm afraid of tightly coupling our "domain model" (reducers and action creators) to our UI, because then changes to one could force changes to the other. Of course, this is speculative and I'm happy to defer this discussion until we've tried it out and can objectively assess the code for strengths and weaknesses.

Injecting Angular dependencies

Just a data point for consideration. I think I would have a harder time tracing the flow of logic if Notify is called within middleware than if it was called directly from within an action creator. The former is implicit whereas the latter is explicit. And readability/clarity aids maintainability, so I think I could find it a simpler migration process in the latter case, too.

weltenwort commented 7 years ago

@cjcenizal please excuse me if I'm temporarily skipping over your comments - I'll get back to them tomorrow morning! :crossed_fingers:

During the discussion that took place in the recent sync meeting I found myself unable to properly verbalize one of the main conceptual problems I have with the approach of eagerly calling asynchronous functions (i.e. returning promises, returning observables, taking callbacks, etc...) in action creators. I would like to give this another try by posing a thought experiment involving the new platform architecture that is to come. Due to the hypothetical nature of the new platform, I will have to make an assumption, which I will try to keep minimal:

ASSUMPTION: Within the new platform architecture, the mechanism by which a plugin is provided with its dependencies requested in the contract is not via static imports but by providing them as argument to an entry point at runtime.

Now imagine that the plugin is an app following the commonly agreed upon React/Redux architecture. This means something like the following exists:

// index.js
import { createStore } from 'redux';
import { render } from 'react-dom';
import { Provider } from 'react-redux';

import { RootComponent } from './views';

export function main(dependencies) {
  const store = createStore(...);

  render(
    <Provider store={store}>
      <RootComponentContainer />
    </Provider>,
    dependencies.rootElement,
  );

  return function destroyApp() {
    unmountComponentAtNode(dependencies.rootElement);
  }
}
// views.js
import React from 'react';
import { connect } from 'react-redux';
import { bindActionCreators } from 'redux'

import { loadObjectFromBackend } from './actions';

function RootComponent(props) {
  return (
    <button onClick={props.loadObjectFromBackend}>Load</button>
  );
}

function mapDispatchToProps(dispatch) {
  return bindActionCreators({
    loadObjectFromBackend
  }, dispatch);
}

export const RootComponentContainer = connect(null, mapDispatchToProps)(RootComponent);

The question I want to pose is: How does the action creator loadObjectFromBackend invoke a function passed from the platform through the dependencies (or interact with an observable passed from the platform through dependencies - whatever the exact interface might be) assuming it can not statically import it (as per the assumption)?

kimjoar commented 7 years ago

++ great example and question, @weltenwort. I think that might be one of the examples that clarifies the potential value this approach can have for our use-case. Definitely things to explore and discuss, but I think it's clear that we should play around with this approach for a bit to get a better feel for it.

weltenwort commented 7 years ago

@cjcenizal here we go: ;)

Group Files by Feature/Domain

I agree that we should take care to avoid such tight coupling. According to my understanding of the redux/react architecture this is how it's done:

Does that sound sensible?

Injecting Angular dependencies

I can see the point about the indirection making it harder to trace the execution path. On the other hand, integrating with external code is what middleware is particularly well suited to do as I explained in one of the rambling comments above. They provide us with a great way to enable action creators to interact with external dependencies without having to jump through hoops to inject them into each and every action creator. The migration process would then just consist of switching out the middlewares with minimal changes to action creators and their tests.

cjcenizal commented 7 years ago

Thanks @weltenwort! Everything you've said sounds good. Let me restrict the scope of my feedback to two very specific things.

UI/View layer

As someone who spends most of his time implementing user interfaces and tweaking user flows, it's really important that I'm able to move large chunks of UI code around, e.g. move a modal from one view to another, or introduce or remove state dependencies in a particular view. So having that level of flexibility and decoupling from the "domain modules" (I like this name a lot btw!), is super critical.

For example, if we had two sibling directories, domain and ui, and we have the domain modules in the former and all files with JSX in them in the latter, then I would be happy. Then I would have a clear entry point into each layer, and I'd be free to organize files within the ui folder, in whichever way best models Kibana from a user's perspective .

Notify

I think my problem isn't so much with your suggestion of using middleware, as it is with the specific example you're using. Our Notify service is a god object which has way too much responsibility (something @epixa mentioned). The design team plans on significantly reworking the way we surface notifications (https://github.com/elastic/kibana/issues/8194), which will impact how Notify is implemented.

I think the most reasonable plan will be to introduce new services which replace specific areas of responsibility, and gradually migrate various views and apps from Notify to these different services. In your example of creating Notify middleware, I would want to increment through our different view files and edit each one individually to use a different notification service. I'm just concerned that this process will be made more difficult if we're implicitly depending upon Notify.

If this makes sense, then how about we just drop the middleware solution for Notify only, but keep it as a tool for other Angular dependencies?

weltenwort commented 7 years ago

separating state / ui layers

I agree that that is an important property to have in our code structure. Let's try to come up with a directory structure that tries to capture that requirement:

I made up the names for the directories on the spot, as always ;) This way, the import graph could look like this (where -> means "imports"):

Platform/Runtime (currently angular routes)
 |
 v
app
 |
 +----------------------------------------+
 |                                        |
 v                                        |
app/containers                           app/{reducers, selectors, actions}
 |                                        |
 +-------------------+                    +----------------+
 |                   |                    |                |
 v                   v                    v                v
app/components ---> common_components    app/modules ---> common_modules
 |   ^               |   ^                |   ^            |   ^
 +---+               +---+                +---+            +---+

In this graph, app/components and common_components would be dumb ui components that should be pretty easy to move around.

weltenwort commented 7 years ago

The description of the different types of notifications in #8194 is nice. We have those types in the context view even now (loading messages, inline-warnings near the buttons and global notifications), so it could serve as a proving ground for that too. I'll try to come up with a proposal for a redux-based implementation.

uboness commented 7 years ago

few comments on my side:

in general... way too deep directory structure... too many directories for single files.. and there's way too much technology specific classification in this structure...I much rather see the functional breakdown then the different parts that make us work with redux.

We're dealing with a very large codebase... that will only grow larger with time - we need to keep things manageable.

weltenwort commented 7 years ago

Thank you for the comments, @uboness!

constants file smell

I personally agree, this can totally be merged into other files like actions.js for the action types, etc. We can allow for some flexibility inside the individual modules as long as the public api exported via the index.js is fully consistent.

no separate modules directory

I just added this to maintain symmetry with the components and containers directories.

no special components directory

A common directory containing the components would work well in conjuction with leaving out the dedicated module directory (see previous point). That would essentially make it a module named common.

no separate containers and components

This part of the proposal was an attempt to accommodate @cjcenizal's request to clearly separate ui from model. If we can further that goal by means of a different convention, absolutely!

too much tech-specific terms

As always, the problem here is finding the right balance between being precise and being abstract when choosing names. The proposed structure tries to accommodate both by applying a functional breakdown on the large (i.e. inter-module) scope and using well-known technical terms in the local (i.e. intra-module) scope. Maybe getting rid of the intermediate directories as described above would already reduce the over-classification sufficiently.

I'd be interested to hear other opinions!

weltenwort commented 7 years ago

(btw, just realized the "directory or single file" description in the proposed directory structure above was easy to misunderstand. What I meant was that this should be a file, but might be split up into a subdirectory if it grows too large. In that case, splitting up the module would probably be the right transformation though.)

uboness commented 7 years ago

As always, the problem here is finding the right balance between being precise and being abstract when choosing names. The proposed structure tries to accommodate both by applying a functional breakdown on the large (i.e. inter-module) scope and using well-known technical terms in the local (i.e. intra-module) scope. Maybe getting rid of the intermediate directories as described above would already reduce the over-classification sufficiently.

agree... will be interesting to see how things look again with the mentioned changes

cjcenizal commented 7 years ago

I like to think of our architecture as layers at a high-level, and concerns at a low-level. In this case, I think of domain as one layer (it represents application state, and encapsulates business logic) and UI as another layer (it's a projection of application state, and it encapsulates UX logic).

Within our domain are concerns that represent how our data is modeled, comprised of reducers, selectors, and actions. Within the UI are concerns that represent the application's UX structure, comprised of containers and components. I'd store all other code, which is framework-agnostic, in a services directory.

So you'd end up with a directory structure like this:

kibana
├── domain (reducers, selectors, and actions -- open for discussion re organization)
│   └── index.js
├── index.js
├── services (framework-agnostic code)
│   ├── api
│   │   ├── some_api.js
│   │   └── index.js
│   └── index.js
└── ui (containers and components)
    ├── index.js
    ├── modals (not tied to any specific page)
    │   ├── confirmation_modal.js
    │   └── index.js
    └── landing_page
        ├── index.js
        ├── landing_page_container.js
        ├── landing_page.js
        ├── landing_page_sub_view_a
        └── landing_page_sub_view_b

I think this is pretty close to @weltenwort's suggestion, and is scalable while also reflecting our architecture.

cjcenizal commented 7 years ago

BTW, we've already started organizing the UI Framework into "components" and "services", so we'll have some consistency if we use this pattern.

uboness commented 7 years ago

@cjcenizal I don't think it's what @weltenwort suggested (if I understood him correctly)... it's actually the other way around. There are (for the sake of this argument) 3 ways to break the directory structure:

  1. based on technical layers (what you suggested)
  2. based on functional layers/modules (what I believe @weltenwort suggested)
  3. a combination of the two... where one is embedded within the other

I'm a big fan of number 2 above. Looking at the kibana module and see services/ui/domain tells me nothing about the architecture or the functional layers. The technical architecture for me is much less important than the functional one. In other words - I'd much rather see "apps", "dashboard", "discover", etc... directories under kibana than "services", "ui", "domain". In each module (say... "dashboard") I might have a DashboardService.js, DashboardPage.js, selectors.js, reducers.js, etc... and an index.js that exposes all the public interfaces for this module... and when you work on dashboards, you only need to deal with this module... not work your way across multiple modules at the top level. This also helps nailing down the module contract appropriately... helps define the boundaries of the module and the directory structure (and thus the JS module structure) should play nicely with these boundaries - there's only one file, one source of truth that defines the public contract of dashboards - it's the index.js under the "dashboard" directory.

btw... the problem I see with option 3 above is what I mentioned above in my discussion with @weltenwort - it gets too complex and too classified - we should keep the directory structure as simple as possible but not simpler.

cjcenizal commented 7 years ago

@uboness I believe I'm actually suggesting #3. In my tree, there are two technical layers (ui and domain), and within each are functional modules. These technical layers are important because they reinforce the relationship between the view and the model -- one is a projection of the other. The domain layer abstracts away complexity and provides a clear API which is consumed by the UI layer.

When you open up a layer, you'll see the concerns you describe, except the concerns you see will be cohesive. When you open up the UI directory, you'll see a map of the application as you're used to experiencing it: home, discover, visualize, management, log_out, fatal_error. Within each directory you'll see the child views of those sections of Kibana. When you open up the domain directory, you'll see an outline of all of Kibana's concerns from a strict domain-modeling point of view: aggregations, index patterns, query language, clusters, user management, and saved objects like dashboards, visualizes, searches.

If we were to put all of these things in a single directory, there wouldn't be any cohesion among the modules. You'd have cross-cutting concerns mixed in with very UI-specific concerns, without a clear idea of what forms our UI and what forms the internal API our UI can consume.

uboness commented 7 years ago

These technical layers are important because they reinforce the relationship between the view and the model -- one is a projection of the other. The domain layer abstracts away complexity and provides a clear API which is consumed by the UI layer.

all this can be done within single functional module - UI, services, selectors (I don't know what "domain" is... everything I just mentioned is part of the dashboard domain) they all relate to each other and each defines as very clear interface... yet they're one thing as a whole.

When you open up a layer, you'll see the concerns you describe, except the concerns you see will be cohesive.

having all services in the app under services is not cohesive... IMO it's a mess, it's mixing completely different unrelated things, just because each one serves a similar role in its existence - but in a completely different context. A service in module X and the role it plays in the context of that module has nothing in common to a service in module Y... the only thing they might have in common is the word "Service" as a suffix - is this a reason to group them together? In the same way, the fact that we're even using a reducer in module X is completely an internal concern to that module - we can have a dashboard app implemented without reducers, without selectors, without redux... we can have it implemented in whatever way we want and it should not be mixed with the implementations of other functional modules. Of course we choose for consistency for all our own implementations, but the fact that these are completely different functional concerns does not change.

If we were to put all of these things in a single directory, there wouldn't be any cohesion among the modules. You'd have cross-cutting concerns mixed in with very UI-specific concerns, without a clear idea of what forms our UI and what forms the internal API our UI can consume.

what cross-cutting concerns? a dashboard service is a cross cutting concern? what's wrong in putting the dashboard service close to the dashboard UI?

I'd *like to have my dashboard service near my UI and near my dashboard related selectors and reducers... definitely. They all relate to one another. why do we need to split dashboards UI form its service? they're dealing with the same thing - dashboard. If we would have (and we're not... but for the sake of argument) to split the whole codebase to modules - I'd not do it to 3 modules - services, domain, and ui... I'd do it to X modules - dashboard, visualise, core, etc... so why would we not structure our code base the same way?

I'll just say that over the years I've seen (and implemented) both ways. IMO, functional modularisation first is the way to go, what happens within each module should be the concern of that module only. I do not need all UI components to be in the same directory to make sense of it (that requirement goes against the notion of a pluggable UI).

cjcenizal commented 7 years ago

The entire store is a cross-cutting concern because this state can be accessed by any part of your UI. For example, the same index pattern state would be surfaced by Management (index pattern listing), Discover (the list of fields in matching indices), and Visualize (the index pattern you've chosen to query against).

uboness commented 7 years ago

The entire store is a cross-cutting concern because this state can be accessed by any part of your UI.

god I hope not.. .that'd be a broken architecture.

everything related to index patterns should be under an index-pattern module that will expose an API for other modules to interact with index patterns. what happens behind that interface - where/why/how/... even whether there's a state store in there or not... none of the business of anyone besides the index-pattern module itself.

cjcenizal commented 7 years ago

god I hope not.. .that'd be a broken architecture.

I hear you... this is just the nature of redux though.

epixa commented 7 years ago

I think there's a misunderstanding at the root of this discussion, so let me try to clarify some things.

The most common use case for redux is for managing the state of an entire UI, so one store per page load. This is how we use redux on cloud, for example, and it is really convenient when it's appropriate.

That said, Kibana itself will not have a single store because Kibana isn't a single application, it's many different applications that integrate with the core UI and each other through the platform itself. Since applications can be built with a wide range of technologies, the "core" logic for managing the state of the world should work in redux, and angular, and ember, and elm, and whatever other framework or technologies people choose to build their plugins with. We could define the interface that all of these technologies must use to interact with Kibana's core state around the redux design/API itself, but we've chosen not to couple these integration points to any specific third party dependency as a fundamental design principle of the new platform.

So Discover becomes a redux application, and it has its own store. And it interfaces with the platform by reacting to changes in the systems it depends on by subscribing to observables and dispatching the relevant actions on its store when those values change.

Visualize becomes a redux application, and it has its own store. Just like Discover, it reacts to changes from the systems it depends on with the whole dispatch song and dance.

Dashboard is an angular app, and it observes changes just like the others, but it sets values on scope and kicks off a digest cycle when it gets new values since redux stores are nowhere in its vernacular.

If there is a shared concept of "index pattern" that is used between these applications, then it must be exposed through the platform and not redux itself. In this case, it would probably be a standalone plugin that is mostly native javascript as well as a small UI for managing index patterns that it registers with the Management application (again, through the platform). That UI may not even be redux because it might be way too tiny to warrant the effort.

In that sense, there is no realistic option as a top level organization strategy other than by-domain, since the highest level of shared state we'll have is the individual applications themselves (discover, visualize, timelion, etc), so they are their own domains. Given that, I think it's going to make a lot more sense to focus on domains for organization rather than trying to hop back and forth between domains and types.

cjcenizal commented 7 years ago

Thanks @epixa... I should have clarified that I was just using Kibana as an example that's both a bit more concrete and also complex. I think we should use the folder structure I'm proposing within the individual app.

Edit: Maybe it would be better if we used a real-life example, e.g. context log?

cjcenizal commented 7 years ago

Here's what I have in mind, using the current Context Log app as an example:

└── context
    ├── domain
    │   ├── index.js
    │   ├── query
    │   │   ├── actions.js
    │   │   ├── index.js
    │   │   ├── reducer.js
    │   │   └── selectors.js
    │   └── query_parameters
    │       ├── actions.js
    │       ├── index.js
    │       ├── reducer.js
    │       └── selectors.js
    ├── index.js
    ├── reducer.js
    ├── selectors.js
    ├── services
    │   ├── api
    │   │   ├── anchor_api.js
    │   │   ├── anchor_api.test.js
    │   │   ├── context_api.js
    │   │   ├── context_api.test.js
    │   │   └── utils
    │   │       ├── sort.js
    │   │       └── sort.test.js
    │   ├── id
    │   │   ├── document_id_factory.js
    │   │   └── document_id_factory.test.js
    │   └── index.js
    ├── store.js
    └── ui
        ├── context_log_controls
        │   ├── context_log_controls.css
        │   ├── context_log_controls.js
        │   └── context_log_controls.test.js
        ├── context_log_size_picker
        │   ├── context_log_size_picker.css
        │   ├── context_log_size_picker.js
        │   └── context_log_size_picker.test.js
        ├── context_log_table
        │   ├── context_log_table.css
        │   ├── context_log_table.js
        │   └── context_log_table.test.js
        ├── context_log_view.js
        ├── context_log_view.test.js
        ├── context_log_view_container.js
        └── context_log_view_container.test.js

This is pretty close to what @weltenwort suggested. I just added a services directory, merged components and containers, and renamed modules to domain.

@uboness Do I understand you correctly that the alternative you're suggesting would be a flatter structure, like this?

└── context
    ├── api
    │   ├── anchor_api.js
    │   ├── anchor_api.test.js
    │   ├── context_api.js
    │   ├── context_api.test.js
    │   └── utils
    │       ├── sort.js
    │       └── sort.test.js
    ├── context_log_controls
    │   ├── context_log_controls.css
    │   ├── context_log_controls.js
    │   └── context_log_controls.test.js
    ├── context_log_size_picker
    │   ├── context_log_size_picker.css
    │   ├── context_log_size_picker.js
    │   └── context_log_size_picker.test.js
    ├── context_log_table
    │   ├── context_log_table.css
    │   ├── context_log_table.js
    │   └── context_log_table.test.js
    ├── context_log_view
    │   ├── context_log_view.js
    │   ├── context_log_view.test.js
    │   ├── context_log_view_container.js
    │   └── context_log_view_container.test.js
    ├── id
    │   ├── document_id_factory.js
    │   └── document_id_factory.test.js
    ├── index.js
    ├── query
    │   ├── actions.js
    │   ├── index.js
    │   ├── reducer.js
    │   └── selectors.js
    ├── query_parameters
    │   ├── actions.js
    │   ├── index.js
    │   ├── reducer.js
    │   └── selectors.js
    ├── reducer.js
    ├── selectors.js
    └── store.js
uboness commented 7 years ago

yes... kibana example was probably the root cause of the confusion.

the example you give is not necessarily what I'm suggesting. I'm suggesting to ask the following questions - what is a service and why do all "services" need to be placed under a "services" directory. What value does it bring aside of adding yet another level in the hierarchy?

within each function module, I don't mind having all ui related code under a "ui"... BUT... a functional module != app module. An app can have multiple functional modules, each defines a set of UIs. The apps we have a big enough where a lot of the arguments I used in the context of the kibana example still hold.

if we keep the functional modules small enough, we should be able to put the services at the top level. Also, I don't like the name "domain"... what is it? It's an overly used term that has different interpretation for different ppl depending where they come from and their experience (what type of applications/system they use to build). What makes a service less of a "domain specific" than a state object. I'd much rather name it what it is - "state".

Another thing.. I also don't think there should be a fix directory structure. If you have a really small functional module - there's no reason not to put most of the file at the top level... only create directories when needed - when there's a real need to group things together for maintenance sake.

bottom line, and sorry for the repetition... it's just really all boils down to this, our code base is huge and it'll only get bigger. Our goal is to keep it a simple as clear as possible - the less files the better (as long as the files don't get too big and too messy - there's a balance there). The less directories the better - as long as it doesn't cause a mess as well... also a balance here.

Have we considered moving the tests out of the main directory structure... I'm not sure what works best in the JS world... I can the the value in having them next to the tested code, they do bring a lot of noise though to the code base.

on a different note.. the following statement:

this is just the nature of redux though.

is exactly what I'm trying to preach against - technology does not and should not dictate the architecture. The technology we use should fit in whatever architecture we put in place for kibana. If there's a mismatch - I'd opt for changing the technology before changing and trying to adapt the architecture. Redux is a choice that we made to avoid the need of implementing something similar ourselves, nothing more than that. The minute someone says something along the lines "I prefer doing X but since I'm using Y I can't and I need to adapt to Y" - there are these options:

weltenwort commented 7 years ago

What @epixa and @uboness argue for makes sense to me. A module's content can be divided into several parts, e.g.:

I am sorry that my ad-hoc choice of the term "domain" caused so much confusion. The intention was to emphasize the fact that the "module boundaries" have been drawn such that they divide along "feature boundaries" and not "technical boundaries". I guess since we pretty much agree that this the type of module we want, we can just as well refer to them as "modules".

To consolidate the requirements and preferences expressed so far: We are trying to map several concepts like

to a directory structure, that has some desirable properties:

So far we seem to agree that

while we have not settled these questions:

cjcenizal commented 7 years ago

What value does [services] bring aside of adding yet another level in the hierarchy?

Where I've previously worked, it was a useful way to share code that cuts across concerns. Perhaps tangentially, it was also a way to deflate directories in the other layers, because the code had no dependencies upon redux or react. Anyway, this is just an option -- we can forgo it initially, and if the need arises for it, it's there for us.

a functional module != app module

I think this is where our shared understanding starts to break down, simply because we don't have the same concrete definitions of these terms. I'm already confused by this distinction. It sounds right, but I don't know if the way I'm interpreting it is the way you intended.

The apps we have a big enough where a lot of the arguments I used in the context of the kibana example still hold.

Exactly why I chose it, but for my arguments. :)

this is just the nature of redux though.

To clarify, I was just trying to answer your question about why state is a cross-cutting concern. Redux exposes the application state as a single interface to the UI (so does Flux, in a way). So if the UI is grouped into directories per page, with reducers in the same directories as the UI code, some pages will find themselves accessing state provided by reducers in sibling directories.

The rest of your points are great, and I agree with them @uboness. I think we're both striving for the same goals, but we're just missing some common context.

@weltenwort, thanks for distilling the problems we're trying to solve.

accommodates several frameworks and libraries like Angular and Redux

I think we should remove this requirement, for now, to simplify the problem.

shared code

Maybe we can also think about how to share code outside of an app in a later step? Once we've found a good place for this code within the app, then I think it would be a small(ish) step to think about the best way to share it.

And just to raise this suggestion again: I feel like we're nearing a point where it would be better to table this discussion until we have an actual working directory structure to critique. If you were to implement Context Log using the standard Redux dir structure, this would establish a good baseline. We all know we want to improve it, we're just trying to come to consensus on how. With some real code to point at, I think it'd be easier to agree on specific problems with the structure, have a share understanding of constraints, and propose changes geared towards each specific problem. Just my 2 cents.

cjcenizal commented 7 years ago

Some nice food for thought from Martin Fowler in 2006: https://martinfowler.com/eaaDev/uiArchs.html. He covers patterns like MVC and MVP, which both strive to separate the domain layer from the presentation layer. I think this is prior art for my proposal to separate our code into distinct domain and UI (presentation) directories. Thoughts?

epixa commented 6 years ago

@Stacey-Gammon Since only apps are using redux and none of the core stuff is, I'm going to throw this over to the apps team

timroes commented 5 years ago

@elastic/kibana-app-arch I think we currently don't have any larger ambitions or needs to setup a styleguide for Redux still. I would close this for now, but if you think you see more apps in the need of this please feel free to reopen.