fix case-sensitive deps not resolving #85
Conversation
|
Yeah sure. I know using lowercase letters is the preferred way, and to me enforcing this rule is no problem at all. But this fix doesn't break anything, passes all the tests. Making some mixed-case minded people happy will do no harm to the current ecosystem. Besides, think of a situation where you want to support both browserify and component builds. Browserify works absolutely fine with mixed-case letters, but component doesn't. And now if you are stuck with a giant lib that has some crazy letters in it, you will most probably drop the component support rather than forking up this beast and maintain component compatibility. As I said, this will do no harm and actually make some good. |
|
okay, good point, thanks. Have no time this week, but I will check it next week |
|
👍 |
|
@timaschew Fixing another issue (infamously @Raynos bug builder2.js/lib/builders/scripts.js Line 323 in 625f3a7 Please do not merge this, not yet, even if you intend to :) I'll send another request |
So this can be closed? |
|
@timaschew have you seen this one? #86 |
|
if #86 is ok i'll merge this with that one |
|
Closing this. See #87 |
Scripts#lookupstarts by lowercasing the target name before doing its job but this causes some trouble if you need to require a file name with some mixed case letters in it.The trouble is that this doesn't resolve to anything:
Now, one way of working around this problem is to lowercase everything first, which in fact what builder is doing right now and most certainly doesn't bring the desired outcome from the developer's perspective. And it doesn't work either, because resolver.js doesn't know that.
A better option is to lowercase only user and repo fragments (this patch)
Since we know user and repo are case insensitive, and must be case insensitive; we can safely translate the reference to lowercase and keep the rest untouched.