Skip to content

Conversation

@mathiasbynens
Copy link
Contributor

@mathiasbynens mathiasbynens commented Jun 10, 2016

This patch updates Punycode.js to v2.0.0. It drops support for old and non-Node environments. This fixes #7224.

@MylesBorins
Copy link
Contributor

lib/punycode.js Outdated
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Out of curiosity, what does this function do to strings like 'foo@bar@baz'? Drop the @baz part?

@MylesBorins
Copy link
Contributor

@bnoordhuis
Copy link
Member

bnoordhuis commented Jun 10, 2016

@mathiasbynens Please steal this patch. I hope the tabs get through alright.

diff --git a/test/message/core_line_numbers.out b/test/message/core_line_numbers.out
index 4cd0c0b..28b4575 100644
--- a/test/message/core_line_numbers.out
+++ b/test/message/core_line_numbers.out
@@ -1,9 +1,9 @@
-punycode.js:67
-               throw new RangeError(errors[type]);
-               ^
+punycode.js:42
+       throw new RangeError(errors[type]);
+       ^

 RangeError: Invalid input
-    at error (punycode.js:67:*)
+    at error (punycode.js:42:*)
     at Object.decode (punycode.js:*:*)
     at Object.<anonymous> (*test*message*core_line_numbers.js:*:*)
     at Module._compile (module.js:*:*)

EDIT: They don't. The spaces before the throw and the caret should be a single tab.

@mathiasbynens
Copy link
Contributor Author

Patch stolen. Thanks, @bnoordhuis!

@MylesBorins
Copy link
Contributor

Punycode v2.0.0 drops support for old and non-Node environments.
@mathiasbynens
Copy link
Contributor Author

I still managed to mess up the tabs somehow, sorry about that. The amended patch passes all tests. Can you kick off CI once more please?

@bnoordhuis
Copy link
Member

bnoordhuis commented Jun 11, 2016

CI: https://ci.nodejs.org/job/node-test-pull-request/2989/ (one of the ppcbe buildbots appears to be stuck so it's possible the run doesn't show up as complete within a reasonable amount of time.)

EDIT: Also, LGTM.

@silverwind
Copy link
Contributor

CI is basically green, LGTM.

@silverwind
Copy link
Contributor

Thanks! Landed in b77eb8c.

@silverwind silverwind closed this Jun 17, 2016
silverwind pushed a commit that referenced this pull request Jun 17, 2016
Punycode v2.0.0 drops support for old and non-Node environments.

PR-URL: #7267
Fixes: #7224
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Roman Reiss <[email protected]>
Fishrock123 pushed a commit that referenced this pull request Jun 27, 2016
Punycode v2.0.0 drops support for old and non-Node environments.

PR-URL: #7267
Fixes: #7224
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Roman Reiss <[email protected]>
@Fishrock123 Fishrock123 mentioned this pull request Jun 27, 2016
Fishrock123 pushed a commit that referenced this pull request Jul 5, 2016
Punycode v2.0.0 drops support for old and non-Node environments.

PR-URL: #7267
Fixes: #7224
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Roman Reiss <[email protected]>
@Fishrock123 Fishrock123 mentioned this pull request Jul 5, 2016
@MylesBorins
Copy link
Contributor

@mathiasbynens @nodejs/lts backport to v4.x?

@mathiasbynens
Copy link
Contributor Author

mathiasbynens commented Jul 12, 2016

@thealphanerd Note that Punycode.js supports Node.js v6+ only. It may work on Node.js v4 as well but no such guarantee is made. It might be better to leave v4.x as-is, especially since Punycode.js v2 doesn’t add any new functionality.

@MylesBorins
Copy link
Contributor

Marking as don't land

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

punycode exports nothing if global AMD define is present

4 participants