Skip to content
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

Replace punycode dep with URL hack #4

Merged
merged 3 commits into from Sep 3, 2019

Conversation

@kanongil
Copy link
Member

kanongil commented Sep 2, 2019

This patch replaces the punycode dependency with a clever hack that uses the WHATWG URL parsing logic to apply punycode decoding to the domain.

I only changed it for the implementation itself, and not the test logic.

I also didn't remove it as a dependency, as it was missing, and only incidentally available in the node_modules from the eslint dependency.

@hueniverse

This comment has been minimized.

Copy link
Member

hueniverse commented Sep 2, 2019

How come the test changed for one of the values?

@Marsup

This comment has been minimized.

Copy link
Member

Marsup commented Sep 2, 2019

@kanongil Have you tested how that impacts joi's browser build ? Also punycode is (so far) part of node, I'm not surprised not to see it in dependencies.

@hueniverse hueniverse merged commit a920a92 into hapijs:master Sep 3, 2019
1 check was pending
1 check was pending
continuous-integration/travis-ci/pr The Travis CI build is in progress
Details
@hueniverse hueniverse self-assigned this Sep 3, 2019
@hueniverse hueniverse added this to the 2.0.1 milestone Sep 3, 2019
@kanongil

This comment has been minimized.

Copy link
Member Author

kanongil commented Sep 3, 2019

The test can be converted to punycode according to https://tools.ietf.org/html/rfc3492, but disallowed for IDNA according to https://tools.ietf.org/html/rfc5891#section-4.2.2. I don't think it matters much.

@hueniverse

This comment has been minimized.

Copy link
Member

hueniverse commented Sep 3, 2019

Agreed.

@hueniverse hueniverse mentioned this pull request Sep 5, 2019
hueniverse added a commit that referenced this pull request Sep 5, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.