componentjs / spec

Component specs
34 stars 4 forks source link

allow defining paths and remotes only on the "main" component #43

Closed jonathanong closed 10 years ago

jonathanong commented 10 years ago

nested paths and remotes are confusing

dominicbarnes commented 10 years ago

I'm torn on this, I use this feature extensively internally when we've separated most everything into components, but I do understand how difficult it is to program and maintain. Since it already works, I say don't remove it, but I'll work with whatever is decided.

ianstormtaylor commented 10 years ago

Id say -1 just cuz it adds an extra exception to the spec. I assume our resolvers should be recursive anyways so they should handle it fine. I think we either let anyone specify paths or get rid of paths altogether

We use multiple paths in our current setup, but it's horrible and moving to just having a single /lib path instead. But not sure is want to impose that with extra logic in the spec

jonathanong commented 10 years ago

Yeah the idea is that you'll only have top level paths, something we all advocate. Having a path that applies only to locals in another path seems like an architecture problem to me. There's no need to do this.

Right now I think we just add use all paths ever defined in any local component, which is incorret. Paths and remotes should only be valid while the component is being resolved, and I already see edge cases I don't want to deal with.

jonathanong commented 10 years ago

errr this is actually less overall logic. checking for the existence of .path in local components is easier than concatenating and such. resolving paths recursively (and correctly) is a lot of additional logic.

my concern isn't the programming logic. this is just something you should never do. imagine if a path is hidden deep inside one of your local components - that would be complicated to debug. easier to have all your paths and remotes at the "root" component.

i can't think of an argument for this. do you guys?

@visionmedia thoughts?

yields commented 10 years ago

-1

jonathanong commented 10 years ago

Reason?

yields commented 10 years ago

i don't think we should enforce people to build their app they way we think is right, i remember a lot of people wanting mvc structure, if they are happy with this structure let them use it...

i think builder should be flexible enough to handle any structure you want.

jonathanong commented 10 years ago

they can build their apps the same. they would just have to define their paths and remotes in the main component only.

ianstormtaylor commented 10 years ago

but forcing everything to be put back at the top-level doesn't sound like a good thing if you're actually trying to use paths for a legit reason :)

jonathanong commented 10 years ago

No one has told me a legit reason though. The only arguments are flexibility, which I'm okay with, but IMO we should keep things simple and add flexibility only when people ask for it and have valid use cases.

ianstormtaylor commented 10 years ago

the problem in my mind is youre adding paths but then adding an arbitrary exception to paths which is going to trip people up, id rather say "paths are something you can use in component.json" than "paths are something you can use in component.json from the top-level component.json you are building from" because then you end up with a docs page that reads like require.js

jonathanong commented 10 years ago

Haha at least they have docs. It's too much marketing though. I have no idea where to look.

That's the point of exceptions though. So developers don't have to read the docs. I didn't read any docs on component until I made this repo.

Plus, I'm going to add a lot of warnings and exceptions already like using old specs, invalid semver range, missing name, etc. I love exceptions. We just need to make them understandable so users can fix the issue without support

tj commented 10 years ago

I agree with it being uber-confusing if they're not defined solely at root, but I also agree with @ianstormtaylor as far as it being extra crap to explain

ianstormtaylor commented 10 years ago

oh sorry, i'm not talking about code exceptions, i'm talking about spec exceptions, like how french (english too) has tons of random ass exceptions which makes learning the language a bitch compared to spanish

stepping back, thats one argument in favor of npm's NODE_PATH= is that it accomplishes the same thing as paths at root, without adding the ambiguity of "can i use paths in all component.json's?"

did we ever settle on paths btw? wasnt there talk of trying to find a better way to do it? like what if we got rid of paths and moved to a --paths or COMPONENT_PATH and then deps looked like:

{
  "dependencies": {
    "remote/component": "0.4.5",
    "local": "*"
  }
}

there are other downsides, like it appearing that locals are versionable. but i'd be in favor of trying to figure out a better way for paths/locals to be handled in general. unless we already decided, i for some reason remember we didn't

i know we're moving to a system where our only path will literally always be /lib or potentially /browser to eliminate the serverside confusion. i wouldn't mind a more restrictive path setup altogether. just thinking outloud

tj commented 10 years ago

the thing I don't like about env vars for that is then it's not described by your app at all, even though it's critical to perform a build, screws me over all the time with NODE_PATH as well

ianstormtaylor commented 10 years ago

yup i totally agree with that

jonathanong commented 10 years ago

closing for now.