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
feat: add additional string prototype methods #4299
Conversation
Codecov Report
@@ Coverage Diff @@
## master #4299 +/- ##
=======================================
Coverage 98.45% 98.45%
=======================================
Files 205 205
Lines 7314 7314
Branches 2083 2083
=======================================
Hits 7201 7201
Misses 55 55
Partials 58 58
Continue to review full report at Codecov.
|
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 you are right, .match
(and .matchAll
) definitely do NOT return booleans. .match
returns either an array with extra properties or null
, and .matchAll
returns an iterator. We should use returnsUnknown
for both.
Similarly, .at
can return undefined
and should therefore also be a returnsUnknown
. Checking the other ...at
methods, .codePointAt
should also be changed to unknown (charAt and charCodeAt seem to return consistent types, though).
Could you adjust those?
Otherwise, I am not sure it makes any sense to mark methods as deprecated, after all, nothing gets every removed from JavaScript so we need to keep supporting it. 😉
ah, ok. I was about to ask. 😄
I'll fix those as well.
the comments are more of a reminder. e.g. |
in the same vein I'm gonna add those other deprecated "HTML" string methods (blink, link, small, etc.), which are still in V8 (incl. node.js) and possibly other engines. |
659a1ae
to
961b84c
Compare
This PR contains:
Are tests included?
Breaking Changes?
List any relevant issue numbers:
Description
@lukastaegert could you look at
match
andmatchAll
ifreturnsBoolean
is what is expected?