Use cliqz-url-parser for URL processing. #410
Merged
Conversation
src/utils/utils.js
Outdated
| protocol: res.protocol ? res.protocol.substr(0, res.protocol.length - 1) : '', | ||
| host: res.hostname || '', | ||
| path: res.pathname ? res.pathname.substr(1) : '', | ||
| host_with_path: (res.host || '') + (res.pathname || ''), |
sammacbeth
Jul 5, 2019
Author
Contributor
This was carried over as-is from the previous version, but I wonder if it intentional that you use host here instead of hostname. The former includes the port number if it is non-standard. Are there patterns that use this information? If not, we could use hostname here to be more consistent. @christophertino
This was carried over as-is from the previous version, but I wonder if it intentional that you use host here instead of hostname. The former includes the port number if it is non-standard. Are there patterns that use this information? If not, we could use hostname here to be more consistent. @christophertino
jsignanini
Jul 11, 2019
Member
I think that since we were creating a somewhat custom object being returned from processUrl(), and not aiming to exactly replicate the URL() function, we just named it host for convenience but I agree that it could lead to confusion.
I think that since we were creating a somewhat custom object being returned from processUrl(), and not aiming to exactly replicate the URL() function, we just named it host for convenience but I agree that it could lead to confusion.
christophertino
merged commit 70a5a72
into
ghostery:develop
1 check passed
1 check passed
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Updates
utils.processUrlto use the cliqz url-parser implementation instead of the current nodeurl.parse. Our benchmarks show this should be able 10x faster than the node implementation.