Closed twolfson closed 9 years ago
Sorry, I gave unwanted metrics before. It is gzipped + minified size we care about:
# With `util/EventEmitter`
$ ./node_modules/.bin/browserify index.js | ./node_modules/.bin/uglifyjs | gzip | wc --bytes
11774
# Without `util/EventEmitter`
$ ./node_modules/.bin/browserify index.js | ./node_modules/.bin/uglifyjs | gzip | wc --bytes
7524
Ah fair enough, I'll give this a shot tonight and let you know. On Jan 21, 2015 4:40 PM, "Todd Wolfson" notifications@github.com wrote:
Sorry, I gave unwanted metrics before. It is gzipped + minified size we care about:
With
util/EventEmitter
$ ./node_modules/.bin/browserify index.js | ./node_modules/.bin/uglifyjs | gzip | wc --bytes 11774
Without
util/EventEmitter
$ ./node_modules/.bin/browserify index.js | ./node_modules/.bin/uglifyjs | gzip | wc --bytes 7524
— Reply to this email directly or view it on GitHub https://github.com/chevett/absolurl/issues/5#issuecomment-70927153.
I already wrote another library actually =/ I will post a link when I am done releasing it.
Ok On Jan 21, 2015 5:01 PM, "Todd Wolfson" notifications@github.com wrote:
I already wrote another library actually =/ I will post a link when I am done releasing it.
— Reply to this email directly or view it on GitHub https://github.com/chevett/absolurl/issues/5#issuecomment-70930703.
it was straightforward to pull out eventemitter. v2.0.0 is available if you'd like.
Glad to hear =)
Our library has been released as well: https://github.com/underdogio/resolve-link
resolve-link doesn't give the results I'd expect with the examples below. Is your intention to build an absolute url?
console.dir(resolveLink('news', 'http://google.com/news/')); console.dir(resolveLink('news', 'google.com/news/')); console.dir(resolveLink('default.aspx', 'google.com/news/')); console.dir(resolveLink('/wtf', 'google.com/news/')); console.dir(resolveLink('file.txt', 'google.com')); console.dir(resolveLink('sky.net', 'http://google.com'));
Yep, the goal is to always build an absolute URL. The second parameter should always be a complete URL.
For reference, these are the results I got:
> console.dir(resolveLink('news', 'http://google.com/news/'));
'http://google.com/news'
> console.dir(resolveLink('news', 'google.com/news/'));
'news'
> console.dir(resolveLink('default.aspx', 'google.com/news/'));
'http://default.aspx/'
> console.dir(resolveLink('/wtf', 'google.com/news/'));
'/wtf'
> console.dir(resolveLink('file.txt', 'google.com'));
'http://file.txt/'
> console.dir(resolveLink('sky.net', 'http://google.com'));
'http://sky.net/'
When I moved all of the second parts to absolute URLs, I got results that I would expect.
> console.dir(resolveLink('news', 'http://google.com/news/'));
'http://google.com/news'
> console.dir(resolveLink('news', 'http://google.com/news/'));
'http://google.com/news'
> console.dir(resolveLink('default.aspx', 'http://google.com/news/'));
'http://default.aspx/'
> console.dir(resolveLink('/wtf', 'http://google.com/news/'));
'http://google.com/wtf'
> console.dir(resolveLink('file.txt', 'http://google.com/'));
'http://file.txt/'
> console.dir(resolveLink('sky.net', 'http://google.com/'));
'http://sky.net/'
oh, i expect different results. It appears that the results you're expecting are exactly what you'd get from the resolve method of the url module.
absolurl
is different from url.resolve
because it has knowledge of tld names and such. For example, it knows that file.txt isn't a valid domain, so it always resolves it as a relative url instead of http://file.txt. absolurl
is of limited benefit, but it is necessary for a silly project that I'm working on.
Your resolution of resolveLink('news', 'http://google.com/news/')
is wrong no matter what a resolver knows. It should be http://google.com/news/news
.
Anyway, it seems that url
or resolve-url
should do what you're looking for.
> console.dir(resolveLink('news', 'http://google.com/news/'));
'http://google.com/news' //this should be 'http://google.com/news/news'
> console.dir(resolveLink('news', 'http://google.com/news')); // removed trailing slash
//should be 'http://google.com/news'
> console.dir(resolveLink('default.aspx', 'http://google.com/news/'));
'http://default.aspx/' //should be 'http://google.com/news/default.aspx'
> console.dir(resolveLink('/wtf', 'http://google.com/news/'));
'http://google.com/wtf' // correct
> console.dir(resolveLink('file.txt', 'http://google.com/'));
'http://file.txt/' //should be 'http://google.com/file.txt'
> console.dir(resolveLink('sky.net', 'http://google.com/'));
'http://sky.net/' // correct
Heh, I thought I was missing a module when I was reviewing my options. I think there is an edge case that I haven't yet reached (e.g. when a username is in a subpath) but I think I will let sleeping dogs lie for now.
Ah, one case I needed a custom library for is when the srcUrl
lacks a protocol. Then node's url
seems to treat it as a path =/
> url.resolve('http://github.com/', 'www.google.com')
'http://github.com/www.google.com'
// We want 'http://www.google.com'
> url.parse('www.google.com')
{ protocol: null,
slashes: null,
auth: null,
host: null,
port: null,
hostname: null,
hash: null,
search: null,
query: null,
pathname: 'www.google.com',
path: 'www.google.com',
href: 'www.google.com' }
i mean, www.google.com
is a valid file name. A dumb file name, but a valid file name. I think the url
module functions as it should. There seems to be a mismatch if you want to handle cases like that, but still resolve to http://file.txt
. I suggest adding a simple check to ensure something is a valid url rather than a new library. As I was saying absolurl
is a silly library, but it attempts examines all valid tlds and such.
I had a check against valid TLDs in the code. However, it was removed because it added 2kb gzipped/minified when it can be added in as an option later on. I decided to not add that because nobody had requested it yet.
https://github.com/underdogio/resolve-link/blob/1.0.0/lib/resolve-link.js#L17-L19
I am very close to using this library. However, it is including a lot of unnecessary weight which is making it a bad idea for the browser (via
browserify
). Theutil
(unused) andEventEmitter
require add on 7kb gzipped.There is no need for
EventEmitter
. This is a synchronous function, not a stream. It should be throwing errors. If it were a stream, then there would bedata
andend
events in addition to theerror
event.