-
Notifications
You must be signed in to change notification settings - Fork 88
refactor: avoid deprecated api #1436
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
base: master
Are you sure you want to change the base?
refactor: avoid deprecated api #1436
Conversation
'url.parse' was deprecated since v11.0.0. Use 'url.URL' constructor instead n/no-deprecated-api
query: | ||
requestUri.search && requestUri.search.length > 0 && requestUri.search[0] === "?" | ||
? requestUri.search.substring(1) | ||
: "", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is a bit clumsy since requestUri.search
has a leading ?
while the old requestUri.query
did not have it
?v=6xJ
vs
v=6xJ
@JKRhb please have a look
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think I ran into similar problems a lot time ago, also in node-coap
:/ Unfortunately, the URL API is really awful IIRC, both when it comes to query parameters and handling IPv6 addresses. In some aspects, it is also not compliant with RFC 3986 if I am not mistaken, which is one reason I didn't transition to the URL API back in the day.
If I hopefully find the time, I will take a closer look at this PR later this week
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just saw that the goals of the "URL standard" include the following:
Align RFC 3986 and RFC 3987 with contemporary implementations and obsolete the RFCs in the process.
That contributes to quite a frustrating experience when working with "proper" URIs using this API. Therefore, we might need to consider using a different implementation here instead (maybe fast-uri
by fastify). Although I am not very happy about that, as I would usually prefer to use the standardized API (if it was actually usable
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I mean, if we fail to use the new libraries, we should stick with what we have...
Mhh, I am not totally sure why it fails on Ubuntu and works on Windows!? |
[::1] vs. [::]
await coapServer.expose(testThing); | ||
|
||
const uri = `coap://[::1]:${coapServer.getPort()}/test/`; | ||
const uri = `coap://[::]:${coapServer.getPort()}/test/`; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Server started with ::
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No matter how I change it, it fails on Linux/Mac 🤷♂️
'url.parse' was deprecated since v11.0.0. Use 'url.URL' constructor instead n/no-deprecated-api
see #1430