componentjs / builder2.js

builder for component
50 stars 20 forks source link

Scripts::lookupRelative fails to find relative module? #17

Closed mnmly closed 10 years ago

mnmly commented 10 years ago

Here's the error dump.

resolver remote not set - defaulting to ["local","github"] +0ms
  resolver:locals resolving local at "/path/to/this/repo" +0ms
  resolver:dependencies resolving dependency visionmedia/debug@0.7.4 +0ms
  resolver:dependencies searching ["local","github"] for visionmedia/debug@0.7.4 +1ms
  builder:scripts no .main found for "lookup-relative-bug" +0ms

/path/to/this/repo/node_modules/co/index.js:308
    throw err;
          ^
Error: could not resolve "./lib/debug" from "visionmedia/debug@0.7.4"'s file "debug.js"
    at Scripts.lookupRelative (/path/to/this/repo/node_modules/component-builder2/lib/builders/scripts.js:267:9)
    at Scripts.lookup (/path/to/this/repo/node_modules/component-builder2/lib/builders/scripts.js:240:12)
    at /path/to/this/repo/node_modules/component-builder2/lib/builders/scripts.js:180:14
    at /path/to/this/repo/node_modules/component-builder2/node_modules/requires/index.js:36:33
    at Array.forEach (native)
    at map (/path/to/this/repo/node_modules/component-builder2/node_modules/requires/index.js:35:17)
    at requires (/path/to/this/repo/node_modules/component-builder2/node_modules/requires/index.js:18:18)
    at Scripts.register (/path/to/this/repo/node_modules/component-builder2/lib/builders/scripts.js:178:8)
    at Scripts.append (/path/to/this/repo/node_modules/component-builder2/lib/builders/scripts.js:156:51)
    at GeneratorFunctionPrototype.next (native)

And here's the demo repo that spits this error. Tried to fix it, but couldn't figure stuff out.

Maybe it's caused by debug which uses condition: typeof window === 'undefined' instead of is-browser. but I thought requiring debug in component.json worked on builder.js (builder1), so put it out here. Let me know if I'm doing some thing wrong...

Thanks.

jonathanong commented 10 years ago

first, i need to fix that error message. it's reporting the wrong filename. haha. should say index.js, not debug.js.

second, it's because it throws if a relative require() lookup fails. i prefer throwing since most of the time it's user errors (i.e. a typo), but i guess sometimes it'll fail because of packages. i should either turn into only a warning or only throw on local component require()s failures. which would you prefer?

i'm going to open a PR to debug though. there's no point in having an index.js.

mnmly commented 10 years ago

Hmm, personally I prefer warning messages just because warning message tend to be used to tell user how to fix it to make this message disappear, but equally make sense to throw on the local lookup too :[

But if you can try PRing to debug, that would be awesome :+1:

mnmly commented 10 years ago

Yep, seems it doesn't throw this specific error :)

But seems there's some issue around toFileObject when running on the demo repo, where it basically complaining there's no .path on obj which should be assigned at this line. But there's no .path on branch object. I tried to debug it by assigning branch.parent.path to obj.path, but still doesn't seem to build right.

Weird that it passes under test :[

/path/to/this/repo/node_modules/co/index.js:308
    throw err;
          ^
TypeError: Arguments to path.join must be strings
    at exports.join (path.js:384:15)
    at toFileObject (/path/to/this/repo/node_modules/component-builder2/lib/builders/scripts.js:120:17)
    at Array.map (native)
    at /path/to/this/repo/node_modules/component-builder2/lib/builders/scripts.js:102:33
    at Array.forEach (native)
    at Scripts.resolve (/path/to/this/repo/node_modules/component-builder2/lib/builders/scripts.js:101:10)
    at GeneratorFunctionPrototype.next (native)
    at Scripts.<anonymous> (/path/to/this/repo/node_modules/component-builder2/lib/builders/builder.js:149:57)
    at GeneratorFunctionPrototype.next (native)
    at Scripts.next (/path/to/this/repo/node_modules/co/index.js:76:21)
jonathanong commented 10 years ago

you gotta update resolver.js and remotes.js. i'm not even using semver ranges on those right now. i guess i should...

jonathanong commented 10 years ago

whoa whoa why do i see node_modules. what sorcery are you practicing?

mnmly commented 10 years ago

Oh, that's the error dump when I run node index.js on the demo repo.

mnmly commented 10 years ago

Maybe I'm just doing something wrong my build script...

jonathanong commented 10 years ago

bump resolver to 0.1.0. maybe use a semver range >= 0.1.0. i don't know how it reaches into node_modules though...

mnmly commented 10 years ago

You are right, pulled the recent ones by npm install component/resolver.js component/remotes.js and builds perfectly :+1: Sorry for the confusion!

jonathanong commented 10 years ago

that repo is helpful. would be nice either as an example/doc here. or maybe just link it in this readme or something. right now i'm just coding it to the point it works - i need more docs. ill take PRs or i can add you as collab

mnmly commented 10 years ago

Okay, I will clean that up a little (and change the name) when I have time then make a PR for adding the link on this README :) Glad that it helped!