kadirahq / mantra

Mantra - An Application Architecture for Meteor
https://kadirahq.github.io/mantra/
976 stars 51 forks source link

What about directory structure? #3

Closed achtan closed 8 years ago

achtan commented 8 years ago

Will be Mantra opinionated about directory structure?

arunoda commented 8 years ago

oh yes.

arunoda commented 8 years ago

Hope this is answered :)

achtan commented 8 years ago

Yes is it, sort of :) Many developers use type-based structure:

actions/
    | articleActions.js
    | commentActions.js
components/
    | article.jsx
    | comment.jsx

imho better is feature-based structure:

comments/
    | comment.js
    | commentActions.js
articles/
    | article.jsx
    | articleActions.jsx

and I'm afraid that Mantra will use first approach and that's kind of worries me :/

arunoda commented 8 years ago

That's something we need to figure out first. We'll go with 1 for now. We will have a module system, where you could keep actions, UI components and the containers in a single folder. You can even distribute them via NPM.

But that's not coming with the initial draft. We'll have it.

natecox commented 8 years ago

+1 for the feature-based structure. I think it's a better way of laying out larger projects, and more in line with the react methodology of grouping feature based code together.

achtan commented 8 years ago

@arunoda you can maybe reopen this issue to get more opinions from people

arunoda commented 8 years ago

@achtan That's a good idea.

leocavalcante commented 8 years ago

Feature-based :+1: This reminds me about Uncle Bob's talks where he says about that architecture should reflect intent like a blueprint of a church/library, you see it and you already know what it is about. You open a source-code folder, sees post/, comment/, author/ and category/ you already knows it's about a Blog.

ghost commented 8 years ago

+1 for Feature based.

Maxximus007 commented 8 years ago

+1 for Feature based

markshust commented 8 years ago

+1 I think it'll be easier to understand logic & testing flow if everything is feature-based

adrianmcli commented 8 years ago

+1 for Featured based.

Agreed. Feature-based is probably better for scalability. I used to use a type-based structure, but an app quickly grows in size and you end up having these huge folders filled with many many files.

arunoda commented 8 years ago

I'm looking to work on the module system. Guys any ideas? @achtan gives us a one solution.

So here's my idea.

In a module, we can have it's own

But only containers and components can be only view to the outside. (Or do we need actions too?) We can only export one container from a each module or many.

If it's a one module we can have a directory structure like this:

* actions.js
* component.jsx
* container.js
* index.js
* tests
    - actions.js
    - component.js
    - container.js

Otherwise, thing will get a bit complicated. What do you think?

natecox commented 8 years ago

One issue that I've had in virtually every framework is how difficult it can be to stay organized when grouping code into single file compartments (e.g., 'actions.js'). My favorite solution to this so far is to turn those files into directories, and inside the directories house a file for each instance.

Taking actions as an example, rather than adding all of the actions for the module into a single file, I would have an actions directory, and each action itself would get a file inside of this directory. This makes it easy to find code you're looking for, makes each file less complicated, and clearly spells out the requirements of each action via imports (if imports are used rather than a global namespace, which seems to be Meteor's preference).

With this in mind, my ideal module would be laid out as follows:

* actions/
    * action1.js
    * action2.js
    * ...
* components/
    * component1.jsx
    * component2.jsx
    * ...
* containers/
    * ...
* tests/
    * actions.js
    * component.js
    * container.js
* index.js

The tests directory is kind of up in the air. I feel like it should be broken down, but I don't know enough about the proposed testing structure and usage to recommend if it should be or not for this use case.

minfawang commented 8 years ago

Very similar to the two above comments, I have a idea writing in this Gist: https://gist.github.com/minfawang/12ca72ac36260426e46b

Within each module/feature, I have "containers, components, actions ...", but rather than just name them as "container.js", "component.jsx", etc, I put them into separate sub-directories. Then each sub-directory has the "index.js" that exports the files/functions/object/anything dependent by other files/modules. So we could still do something like import { addTodo } from 'TodoApp/client/actions'

rnarayan commented 8 years ago

+1 for the feature based structure recommended by natecox.

achtan commented 8 years ago

@natecox and @minfawang your proposals are type-based in context of module...

@arunoda agree with you, that's a good direction! btw: but i think we can also export some of the actions if it is necessary. For example if you have cart module in eshop maybe you will need some actions to by accessible in other modules or so...

arunoda commented 8 years ago

@achtan Yeah! That's a good one. May be there are some modules, which only have actions.

There is one thing I need to clarify. We won't have a server directory inside this

We need to make sure client and server are two separate things. But it's okay to have method_stubs like this.

minfawang commented 8 years ago

@achtan Oh my bad.. Sorry for not reading the post carefully. Now I see your point.

I think both approaches have its own advantages. For the "one module" approach proposed by @arunoda I think the root directory can get cumbersome when the app grows to a large scale.

I feel actually my approach could be thought as feature and type combined based if you separate modules based on features. Currently I separate modules as different code splitting points. You can think of a module as one big feature or a set of small features.

natecox commented 8 years ago

@achtan This was within the given context of 'structure inside a module'. If you assume that modules are based on features, than this would definitely be feature based rather than type based.

Type based would be

*components
    * Todo
    * Different Todo
* actions
    * Todo
    * Different Todo

where we're suggesting

* Todo
    * components/
        * ...
    * actions/
        * ...
* Different Todo
    * components/ 
        * ...
    * actions/
        * ...
natecox commented 8 years ago

@arunoda One thing I noticed while reading the spec is that the suggestion is to put all of your components inside the client/ directory. How are you handling server side rendering for SEO in this context? I've always had to provide my components via lib/ to make them accessible for SSR.

adrianmcli commented 8 years ago

With larger apps, things tend to get complicated very fast. Files get bloated quickly, and I always feel like I want to make many smaller components rather than fewer larger ones.

What do you guys think about modules with sub-modules approach?

- ChatModule
    | component.js
    | actions.js
    | container.js
    - ChatMsgsArea/
        | component.js
        | actions.js
        | container.js
    - ChatInputArea/
        | component.js
        | actions.js
        | container.js

- SettingsModule
    | component.js
    | actions.js
    | container.js
    - GeneralSettings/
        | component.js
        | actions.js
        | container.js
    - AdvancedSettings/
        | component.js
        | actions.js
        | container.js
arunoda commented 8 years ago

@adrianmc I think we should limit to a single level of modules. Otherwise, we'll have a lot of nested modules and very hard to deal with.

natecox commented 8 years ago

@adrianmc I feel like submodules make it too easy to violate the single responsibility principle.

achtan commented 8 years ago

@natecox if you isolate this one:

* Todo
    * components/
        * ... // what will be here? todoList.js, addTodoForm.js
    * actions/
        * ... // and here? fetchTodos.js, addTodo.js

it is type base structure inside module because you separating code by type

achtan commented 8 years ago

maybe a good question would by "what is the 'Module' and what you / I / everyone think under that term"

arunoda commented 8 years ago

@natecox Actually, that's a decision of Meteor's current 1.3's layout system let's talk about SSR here. See: https://github.com/kadirahq/mantra/issues/11

natecox commented 8 years ago

@achtan You're right, if you look at only the inside of the structure it would be type based, but that's kind of a moot point in my opinion given the context here is _what to do after you've grouped your code by feature_.

I view it this way: I break my project (the entire working site) into modules (specific features of the site), and then the modules themselves into types.

From your earlier post, you're essentially doing the same thing, referencing

comments/
    | comment.js
    | commentActions.js
articles/
    | article.jsx
    | articleActions.jsx

Where 'Action' could be defined as a type as well. I'm just going a step further and making them a directory to make each file single purpose and less verbose.

arunoda commented 8 years ago

@achtan @natecox let's deal with that first.

What is a module

  • I think it's better to have a single layer of modules. Nested modules always make it hard to reason and one way to break the simplicity of the app.
  • Since we don't have versions of modules (no we don't have it) it's safe to avoid nested modules.
  • Modules can read from the application context. But they can't write into that.
achtan commented 8 years ago

@natecox but I see your point! lets talk about some real-world example: eshop + blog so now we can say that eshop is an module and also the blog

eshopModule/
    | config/ 
    | lib/
    | *not sure how to name this dir* /
        | cart /
            | actions.js
            | component.js
        | goodsList /
            | actions.js
            | compnent.js
    | main.js
blogModule/
    | config/ 
    | lib/
    | *not sure how to name this dir* /
        | article /
            | actions.js
            | component.js
        | articlesList /
            | actions.js
            | compnent.js
    | main.js

i don't see the point to spliting eshopModule/**/cart/actions.js into subdir... if you have too much actions in it, its probably too big feature and you should split into several smaller

natecox commented 8 years ago

@achtan I think what you've got there is kind of enforcing submodules. Maybe moving things up a level would help:

cart/
    | components/
        | CartItem.jsx
        | CartList.jsx
        | PaymentMethod.jsx
    | actions/
        | cart-management.js
        | checkout.js
    | config.js
    | main.js
inventory/
    | components/
        | SundryItem.jsx
        | SundriesList.jsx
    | actions/
        | inventory.js
        | restock.js
    | config.js
    | main.js
blog/
    | components/
        | Article.jsx
        | ArticleList.jsx
    | actions/
        | article-crud.js
    | config.js
    | main.js

I absolutely think that components should be split up. By design React already has a fairly complicated structure, because you're inherently grouping together design and functionality (which is great, but increases code in one location). In my experience, it's much easier to work with components if they're limited to a single component per file.

Maintaining convention between component structure and action structure encourages that single purpose principle's usage, which is a positive to me. However, I see the argument as well that there shouldn't' be all that much in actions by design, so I could take it either way.

xavierpriour commented 8 years ago

As another example, here's what I have that make up a logical "modules/features" - currently scattered across many directories:

And what I would leave out of modules:

I'm wondering how Mantra will handle the server/client stuff - any idea yet? Should it be a new Github issue?

arunoda commented 8 years ago

@xavierpriour Mantra sees both client and server as a two separate places. Have a look at the spec and the sample app.

natecox commented 8 years ago

@achtan Good discussion so far. I'm heading to bed and will check on this again in the morning (it's like 3am here).

@arunoda Seems like there's some good community involvement incoming. You should consider getting a slack group opened for this project, or some other alternative.

arunoda commented 8 years ago

@natecox I don't like chat systems since whole communication get lost and not public. Let's try to keep it here. If we need more, I'll create a discourse server.

xavierpriour commented 8 years ago

@arunoda ok, sorry I had missed it. Should a Mantra module be able to contain both server and client code? Or is it just client-side?

arunoda commented 8 years ago

@xavierpriour Just client side. We need to separate the server. That's the future specially with stuffs like React Native, GraphQL (declaritive way to defining data) and more reasons to tell :)

xavierpriour commented 8 years ago

@arunoda oh ok - mind if I update the spec on Modules to make it explicit they're client-side only?

appinteractive commented 8 years ago

@arunoda will there be a way to use mantra/modules in packages? on big project you want to bundle feature based functionality, this means client and server code in case of meteor.

arunoda commented 8 years ago

@xavierpriour it's not only modules but everything. Yeah. Send me a PR.

arunoda commented 8 years ago

@appinteractive That's the whole point in this thread. We are collecting ideas. But that's only for the client. Modules can be distributed via NPM. (not via Atmosphere)

markshust commented 8 years ago

@arunoda I thought I heard this a few places, but is Meteor (and/or Mantra) not recommending Atmosphere at all anymore, and switching everything to NPM? I'm assuming so, but wanted to confirm so I can start preparing for that.

arunoda commented 8 years ago

Yes. Atmosphere will die sooner or later. It's not a bad thing. It will take a time to happen.

That's why we are staying out of it. On 2016 ජන 12, අඟහ at ප.ව. 8.45 Mark Shust notifications@github.com wrote:

@arunoda https://github.com/arunoda I thought I heard this a few places, but is Meteor (and/or Mantra) not recommending Atmosphere at all anymore, and switching everything to NPM? I'm assuming so, but wanted to confirm so I can start preparing for that.

— Reply to this email directly or view it on GitHub https://github.com/kadirahq/mantra/issues/3#issuecomment-170943328.

makstr commented 8 years ago

@arunoda thank you for Mantra, another great community contribution!

As to the directory structure and module separation I adopted this pattern pattern in Meteor when using kickstart-hugeapp. This boilerplate has "modules" folder and each subfolder can contain its own separate client and server directories. It's pretty neat.

But my understanding is that in Mantra we need to keep the server separate, is that correct?

arunoda commented 8 years ago

@MaksTr That's what's I'm planning to do without the server part.

luisherranz commented 8 years ago

@arunoda what's your thought on modules which needs client and server code? For example, they need to define a Meteor.method in the server:

cart/
    | client/
      | components/
          | CartItem.jsx
          | CartList.jsx
          | PaymentMethod.jsx
      | actions/
          | cart-management.js
          | checkout.js
      | config.js
      | main.js
    | server/
      | cart-methods.js
arunoda commented 8 years ago

@luisherranz we really need to keep server code away from here. If we need to get latency compensation support, we can define the method stub here on the client like this. See: https://github.com/mantrajs/mantra-sample-blog-app/tree/master/client/configs/method_stubs

If needed, you can put these methods on a common directory and import on both client and the server. But for Mantra it's just a configuration.

luisherranz commented 8 years ago

Ok, so you mean Mantra needs a root client and a root server folder and those can't be mixed up inside modules, is that right?

arunoda commented 8 years ago

Exactly. I'm working on a new update to the spec the module system and some other improvements. Will publish it on monday.

arunoda commented 8 years ago

@luisherranz If you are confused about this decision. It has a reason. Mantra believe in isomorphic concepts/modules rather isomorphic apps. May be it's a good topic for a blog post :)