componentjs / builder2.js

builder for component
50 stars 20 forks source link

Add support for nested local dependencies #75

Closed bowd closed 10 years ago

bowd commented 10 years ago

We want to be able to require local components that are nested inside of the defined paths.

So, for example, instead of having the base component.json paths be ['actions', 'stores']. And then (because the components would live in the same namespace), have the components:

And do: require('modal_actions') or require('modal_stores').


We want to be able to have the component.json path be ['./'] (example). The components would be namespaced by the folder so:

And then do: require('actions/modal') or require('stores/modal)`

PS: there's also a new fixture in the pull-request, called js-nested-locals, which may help in understanding where we're coming from.


Currently that's not possible because of lookup for local dependencies:

  if (branch.locals[target]) return branch.locals[target].canonical + tail;

Which actually ends up doing something weird. At the beginning of the function it splits the target:

var frags = target.split('/');
var reference = frags[0];
var tail = frags.length > 1
  ? ('/' + frags.slice(1).join('/'))
  : ''

And then it looks up the locals by target but it still adds the tail. So when running our example without our fix, requiring something like nested/boot ended up being rewritten as nested/boot/boot.

So even without our fix, it feels like the lookup should happen by reference and the tail would be added to that.


The only drawback to this approach is that you actually have to set the name of the nested components including the path in component.json, so (like you can see in the example) boot's name is nested/boot inside component.json.

bowd commented 10 years ago

I think it's also relevant to #46 / #48

bowd commented 10 years ago

@jonathanong hey there. I don't wanna sound aggressive, cose' I know it's been a short time since I made the pull-req, but I'm wondering if component will still live on now that Duo came out of the closet.

We still have a custom-ish build tool on top of component-resolve and component-builder that we're fairly fond of, and it's working for us really good right now, so we might stick with it until it makes sense to move.

So basically my question is. Is it worth setting up a fork or will these repos be at least sort of actively maintained?

timaschew commented 10 years ago

I will check your PR soon :)

It's not clear about the maintenance for component, because duo is not ready yet and it's also no guarantee that it will solve all problems of component and will redeem component: https://github.com/duojs/duo/issues/255

But in the meanwhile some of us like @clintwood are trying to fix issues for component

bowd commented 10 years ago

Cool @timaschew thanks a lot for the heads up. I'm curious to see how this will evolve.

clintwood commented 10 years ago

@bogdan-dumitru, as @timaschew said I don't think there is much mileage in adding 'features' to component right now... Lets see how things progress... I would suggest you try see if Duo would work for you and try add your feature requests there...

bowd commented 10 years ago

@clintwood point taken, but I'm curious what your definition of a feature request is, because I think that this pull-requests solves an issue that I found in component. And by also providing tests and a proposed implementation of the fix, it might qualify as more than a feature request, but then again that's just me.

clintwood commented 10 years ago

@bogdan-dumitru, agreed, perhaps wrong words...

I did take a quick look at your PR and to me it looks good, @jonathanong, any side-effects you can see if PR gets merged?

timaschew commented 9 years ago

@bogdan-dumitru

The only drawback to this approach is that you actually have to set the name of the nested components including the path in component.json, so (like you can see in the example) boot's name is nested/boot inside component.json.

I fixed this in the resolver, so you can omit the name property now ;)