-
Notifications
You must be signed in to change notification settings - Fork 8
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Deno compatability? #27
Comments
@viztastic I’m open to your proposal but I don’t have any experience with deno. If you can provide a PR with the necessary changes, I’m happy to merge it. |
No problems @AlexZeitler - I've just had a go at aligning the module in a fork. I'm just dealing with one error: Node JS has deprecated I had a very naive idea which was to check if the instance had a base path, and if it didn't, we insert a fake one and then remove it again... but then also what if we just stored the string directly, is that bad? |
@viztastic We could default to http://localhost or http://tempuri.org - does this make sense to you? |
Or should we expect a valid URL including a base URL? |
I don’t mind forcing the base path but I’m worried it won’t be compliant with the spec because they explicitly support without the base path. Maybe we ask the user to include it but they another option to strip out the base? |
I think I would do something like this then: yarn add is-relative-url import isRelativeUrl from 'is-relative-url'
const getUrl = (url: string) => {
return isRelativeUrl(url) ? getRelativeUrl(url) : url
}
const getRelativeUrl = (path: string) => {
const url = new URL(path, 'http://tempuri.org')
const { pathname, search, hash } = url
const result = pathname + search + hash
return result
}
// relative
const url = '/test?test=test#test'
const result = getUrl(url.toString())
result.should.equal('/test?test=test#test')
// absolute
const url = new URL('/test?test=test#test', 'http://tempuri.org')
const result = getUrl(url.toString())
result.should.equal('http://tempuri.org/test?test=test#test') |
Yep that makes a lot of sense 👌 |
Ok, I build a npm package for it ( |
Is it possible to include an export map and entry point to make this library deno friendly?
We could even add it to the deno registry that way and make the library the default for Deno's equivalent to express (Oak).
Thanks so much!
The text was updated successfully, but these errors were encountered: