componentjs / spec

Component specs
34 stars 4 forks source link

relative locals #26

Closed jonathanong closed 10 years ago

jonathanong commented 10 years ago

ex. for boot, you would currently do:

{
  "paths": ["client"],
  "local": ["boot"]
}

would be nice if you could just do:

{
  "local": ["client/boot"]
}

i feel like if you have too many paths, you could basically run into global conflicts. i'd like to have model/thing and view/thing for example without doing component names like model-thing and view-thing.

also, why is it called local? should be locals since it's an array.

ianstormtaylor commented 10 years ago

+1 to locals (separate issue?)

i think having too many paths is an anti-pattern. or maybe it only applies to too many paths that contain ../ which is the hellhole that i need to get the segment.io frontend out of :)

dominicbarnes commented 10 years ago

I've always wanted ./ to be an implied path, so +1

ianstormtaylor commented 10 years ago

implied path is cool idea. too bad implying ./lib seems like it might be stretching it :) but ./ is still interesting

tj commented 10 years ago

yeah more than one lookup path sucks haha, i think more than one level of nesting in any application is an anti-pattern

jonathanong commented 10 years ago

i'm pretty confused now because you are guys are just calling things anti patterns without thorough explanations. hate when this stuff happens on twitter.

i agree that something like client/module/submodule/index.js is anti-pattern, mostly because is just really difficult to navigate the source code. i mean, look at ghost's source code. i can't find anything. i think this is one of the main benefits of components that we should market.

however, i don't see how this relates to paths. i would consider this good structure:

client-view/
  people/
    component.json
  place/
    component.json
  component.json
client-model/
  people/
    component.json
  place/
    component.json
  component.json

since you're separating logic from presentation, but you'll need a lot of paths/locals magic here. i think it'll be better if we can do var People = require('model').people from within the view instead of naming everything people-view and people-model like in the todo example. this is wayyy better than node (or js in general) where you'll have stuff like require('../../../../../../lib/utils').thing, and you'll get so annoyed you just make a global.

now, are we talking about something else? what exact structure do you consider an anti pattern?

ianstormtaylor commented 10 years ago

I'd disagree about it being better than naming things "people-model". That's exactly what it should be named because that describes t perfectly. We made this mistake and one of my next tasks is to rename them all to make it more clear.

But also i agree with TJ that having more lookup paths than a single "./lib" folder is usually bad. (Obviously "./components" excluded.) We also used to do this, and it just gets too magicky for everyone else reading to have to look through all the component.jsons to figure out what's happening. The "downside" people bring up of havin a huge lib folder is nowhere near the downside of having too many folders in terms of maintenance.

The other thing were doing now is that we will only have local lib folders for individual express sub apps (one per page of the site generally). There is no common shared "./lib" among them because then you need to reach out, and none of the shared stuff will be versions, so you end up in a shit hole again.

jonathanong commented 10 years ago

so you don't separate all your views from your model? that sounds terrible to me. for simple stuff that should be fine, but the "downside" is more significant to me. you're mixing your model, controllers, and views all in the same folder (though I hate those terms). i agree that multiple component paths are bad, but i disagree that multiple folders are bad.

i would say having a express sub app per page is an anti pattern - you're bringing guns to knife fights. we should probably decouple views from apps in the future so you don't have to do this (or just use co-view). i think the only time you should be using sub apps is when it's actually a subapp - like mounting a blog on merchant site.

so the goal of relative locals is to avoid look up paths. there will be fewer paths, which i think we all agree having more than one is generally bad. for me, more than 1 look up path is an anti-pattern on a per-component basis, but it sounds like you guys think this is true for the entire app. are we okay with this?

if you do something like:

{
  "name": "app",
  "locals": ["./client/person"]
}

within app, you should be able to require(name) where name is ./client/person/component.json's name. relative locals must begin with a ./.

@dominicbarnes what do you mean by implied path?

dominicbarnes commented 10 years ago

@jonathanong I mean that the component root directory is always assumed to be a path to load locals from.

Right now, to get your last example to work, you'd need to add paths: [ "./" ].

ianstormtaylor commented 10 years ago

we don't combine views and models, our directory structure would look like this:

lib/
  integration-model/
  integration-tile-view/
  integration-editor-view/
  integration-form-view/
  org-model/
  org-view/
  user-model/
  user-view/

my opinion is that paths should always be ['./components', './lib'] and nothing more to keep a maintainable system. although i could be convinced that ./ is also in the group.

i don't think subapps is a problem, or at least i don't have enough experience yet. it's not that every page needs to be a subapp, just that like pages are combined. and in the case of marketing pages they are often unlike all the others because you want them self contained, so in some cases it does become a 1:1 subapp:page mapping. but in the case of login then forgot password and stuff are also included in there

jonathanong commented 10 years ago

Less about app structure, more about specs. Relative locals?

ianstormtaylor commented 10 years ago

-1 because it results in a crappy app structure

jonathanong commented 10 years ago

Actually, yeah. None of my arguments make sense if we just add ./ to that paths. Nice.