componentjs / builder2.js

builder for component
50 stars 20 forks source link

local component files can't be `require`d #46

Closed azhang closed 9 years ago

azhang commented 10 years ago

The builder successfully resolves

require('local-thing/file');

but in the browser, it throws:

Uncaught Error: failed to require "local-thing/file"
azhang commented 10 years ago

Actually it's the builder not recognizing local-thing as a local and thus doesn't replace it with the correct path.

jonathanong commented 10 years ago

can you give me a test case? afaik you could be doing something else wrong.

azhang commented 10 years ago

Sure. making them now. I also noticed that many of the test cases mix ' and " which might be an issue. In my test case, I'm noticing that even though in the file I have

require('subcomponent-1/hello')

the built js shows

require("subcomponent-1/hello")

and thus have to test for

js.should.not.include('require("subcomponent-1/hello")')
jonathanong commented 10 years ago

https://github.com/visionmedia/node-requires always rewrites to double quotes

azhang commented 10 years ago

Not sure why github didn't link automatically - https://github.com/component/builder2.js/pull/48

azhang commented 10 years ago

Actually I think this is the same issue as #30 . The tests I added demonstrate how this fails

timaschew commented 10 years ago

require('local-thing/file');

should this work without file extension?

clintwood commented 10 years ago

This works for me... but only if I prefix with ./ like require('./local-thing/file'); and include it in component.json like"scripts": ["local-thing/file.js"]... HTH.

timaschew commented 10 years ago

This works, but it breaks your self contained module, because it's not just a ./ it's the .path property of the root components component.json, where component try to find locals.

A local module should know nothing about this path.

So this is more a hack than a solution.

clintwood commented 10 years ago

@timaschew, true, but local components are, IMO, a hack anyway and I only really use them because it's so intense to put every little component into a github style repo (even our in-house github style component server)... it's a pain... So I live with known physical structure on the file system... then once they mature, publish then to our github style server...

I'm really hoping Duo solves this issue... (@MatthewMueller ?)

timaschew commented 10 years ago

why it's a hack to use local components?

clintwood commented 10 years ago

I don't want to start a war here but for us, mainly because of what you pointed out above... for one, where local components depend on other local components the component.json may need paths and does need locals to point to their dependent components. This causes a local file system structure dependency, there is no versioning, etc. Then when promoting local components to being repo based they need all their local dependencies to be pushed to being repo based and you then need to remove the paths, locals etc and update all referring component dependencies. For a system with many components this become a high maintenance task...

Also, I should mention that our workflow is to start a components life as a local component then promote it to a repo based component once it is stable... Also, because we share components across different applications we tend to opt for breaking things into many reusable components...

azhang commented 10 years ago

For us, we put the whole project into a single repo and use local components to organize code. We use locals only for code specific to the project - anything reusable we start it off as a separate component.

matthewmueller commented 10 years ago

@clintwood there's no concept of local components in duo. It's all relative paths require('./a.js), or resolving from project root require('/a/a.js').

At cloudup, I found it incredibly confusing to differentiate between local and remote components. we had about 40 deps at the top of some of our files and it wasn't always clear where these deps lived.

The reason for locals in component was the thought that relative paths added noise. Personally, while adding relative paths adds a few more characters, I think it's a lot more clear what's going on. The other argument for locals was that it's super easy to turn them into remote components. While this is true, it's not difficult to add a component.json to a local component when you want to open source it.

Also, with duo, you can specify paths on remote repos require('component/tip:index.js') so you don't even need to open source with a component.json though I would recommend it :-)

clintwood commented 10 years ago

@aaronz8, that's pretty much how we have it... only thing is moving a local component to remote component is a pain!

@MatthewMueller, thanks... so to promote a local component to remote would be a global search and replace through a projects code base from say require('../local/my-component-x'); to require('myrepo/my-component-x@0.0.2');. nice...

matthewmueller commented 10 years ago

@clintwood yep, exactly. you might need to add a component.json to require('../local/my-component-x'); though if your entry was an index.js it would pick it up anyway.

timaschew commented 10 years ago

@clintwood

Then when promoting local components to being repo based they need all their local dependencies to be pushed to being repo based and you then need to remove the paths, locals etc and update all referring component dependencies. For a system with many components this become a high maintenance task...

so to promote a local component to remote would be a global search and replace through a projects code base from say require('../local/my-component-x'); to require('myrepo/my-component-x@0.0.2');

What if there is a require('../../local/my-component-x') - in that case you global replace would fail. Okay, you would be fine with a regex then, but I think you can also solve it for local components with a small tool, which analyze your require calls and update them inclusive the component.json files for the paths and locals properties.

clintwood commented 10 years ago

@timaschew, yes true... Actually, if we could get away from a component.json manifest world, life would be better... so here is an ideal scenario: I like to break things into components, both public and private, I'd like to start off components in a git repo.

So this could work with Duo, I'm hoping, (with a local-file-system-git-repo-resolver, hint, hint @MatthewMueller).

I believe we should only ever require components like this var foo = require('namespace/repo@semver');, a well understood signature. During initial development semver would mainly be master but could be any valid semver. So with a local-file-system-git-repo-resolver in Duo's pipeline it could probe local git repos first before heading off to GitHub and friends. If I wanted to promote this repo to an inhouse git server or GitHub or whatever I could do global search and replace of namespace/repo@semver and rename the namespace, repo or both... minimum effort, very robust! The required component itself would use the same scheme for requiring its depenant components and there would be no need for a manifest!

Some simple tooling will be needed to walk the dependencies in a project and even potentially reduce the effort of typing (or remembering the version of) namespace/repo@semver for a project...

Local files etc. can be required like this var bar = require('./some/file.js); and variants...

I think I'm way off topic here... :)

jonathanong commented 10 years ago

that's basically what normalize does right now

clintwood commented 10 years ago

@jonathanong, hahaha, thanks, that occurred to me right after posting... will take a better look at it...

timaschew commented 9 years ago

close due to https://github.com/componentjs/builder2.js/issues/82