url,tools,benchmark: replace deprecated substr()#51546
Conversation
|
Review requested:
|
|
If we are doing this, we should add a ESLint rule to avoid making the same decision over and over again. |
7242e8c to
879629c
Compare
|
@anonrig Thank you for your good opinion. |
lib/url.js
Outdated
|
|
||
| if (hasTrailingSlash && (srcPath.join('/').substr(-1) !== '/')) { | ||
| const srcPathSlashes = srcPath.join('/'); | ||
| if (hasTrailingSlash && (srcPathSlashes.substring(srcPathSlashes.length - 1) !== '/')) { |
There was a problem hiding this comment.
shouldn't this be srcPathSlashes.charAt(-1) instead of srcPathSlashes.substring(srcPathSlashes.length - 1)?
There was a problem hiding this comment.
srcPathSlashes.charAt(srcPathSlashes.length -1)
There was a problem hiding this comment.
srcPathSlashes.charAt(srcPathSlashes.length -1)
hmm I mean sure... but .charAt(-1) accomplishes the same result
There was a problem hiding this comment.
.charAt(-1) works differently than before. I think .slice(-1) is correct for this case. Thank you for your opinion.
|
I had divided the commits for review, but I will merge them into one and edit the First commit message. |
1b33e8a to
07f751c
Compare
substr() -> substring()substr()
07f751c to
094c72d
Compare
|
Landed in 78dbda1 |
PR-URL: #51546 Refs: https://developer.mozilla.org/ko/docs/Web/JavaScript/Reference/Global_Objects/String/substr Reviewed-By: Jithil P Ponnan <jithil@outlook.com> Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
PR-URL: #51546 Refs: https://developer.mozilla.org/ko/docs/Web/JavaScript/Reference/Global_Objects/String/substr Reviewed-By: Jithil P Ponnan <jithil@outlook.com> Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
PR-URL: #51546 Refs: https://developer.mozilla.org/ko/docs/Web/JavaScript/Reference/Global_Objects/String/substr Reviewed-By: Jithil P Ponnan <jithil@outlook.com> Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
PR-URL: #51546 Refs: https://developer.mozilla.org/ko/docs/Web/JavaScript/Reference/Global_Objects/String/substr Reviewed-By: Jithil P Ponnan <jithil@outlook.com> Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
PR-URL: nodejs#51546 Refs: https://developer.mozilla.org/ko/docs/Web/JavaScript/Reference/Global_Objects/String/substr Reviewed-By: Jithil P Ponnan <jithil@outlook.com> Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
PR-URL: nodejs#51546 Refs: https://developer.mozilla.org/ko/docs/Web/JavaScript/Reference/Global_Objects/String/substr Reviewed-By: Jithil P Ponnan <jithil@outlook.com> Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Replace deprecated API.
Refs: https://developer.mozilla.org/ko/docs/Web/JavaScript/Reference/Global_Objects/String/substr