added ts definitions #29
Conversation
|
Hi @Akim95, thanks for the contribution. I haven't used TS before. Can you clarify for me the maintenance burden this would introduce if I were to modify the API? Is the TS file automatically updated or would new PRs need to update it if they changed the API? |
index.d.ts
Outdated
| limit: number; | ||
| } | ||
|
|
||
| declare function safeRegex<T>(regex: RegExp, opts?: Options | T): void; |
There was a problem hiding this comment.
- currently the safeRegex function takes
RegExp's ,stringsand anything with a toString method in its re parameter (so technicallyany, I guess). Which'd make this declaration a bit too restrictive. - this definition seems to have
voidas a return type - but the function currently returns a boolean (A void return type would've been correct if safeRegex would've been implemented as a function that threw an exception in case of problems and silently exited in all other cases). - I see you're using a generic (the
<T>&Tin the declaration - had to look that one up) why is that?
Need a new PRs but no need to worry if you have any API need to change because people that using this package with Typescript will taking care of it. |
| @@ -4,6 +4,6 @@ interface Options { | |||
| limit: number; | |||
| } | |||
|
|
|||
| declare function safeRegex<T>(regex: RegExp, opts?: Options | T): void; | |||
| declare function safeRegex<T>(regex: RegExp | string | T, options?: Options): boolean; | |||
There was a problem hiding this comment.
- The union for the regex with RegExp and string is an improvement 👍
- Could you explain how the generic (the
<T>and theTin union of the first parameter) relates to the original function? The declaration without it (export function safeRegex (regex: RegExp | string | any, options?: Options): boolean) seems to match the original function just as well, but I might've missed some thing.
|
For the record, I believe most people are using the definitions in DefinitelyTyped for this package. |
|
Just found this package recently and I'd like to use it with TS definitions. Are there plans to keep this PR up to date and merge it? I can open a new one if the original author isn't available anymore @davisjam |
|
Have there been any changes since the original PR was opened? Maybe it's ready to merge now. Can you please let me know?
Get Outlook for iOS<https://aka.ms/o0ukef>
…________________________________
From: Rodry ***@***.***>
Sent: Monday, October 24, 2022 5:57:12 PM
To: davisjam/safe-regex ***@***.***>
Cc: Davis, James C ***@***.***>; Mention ***@***.***>
Subject: Re: [davisjam/safe-regex] added ts definitions (#29)
---- External Email: Use caution with attachments, links, or sharing data ----
Just found this package recently and I'd like to use it with TS definitions. Are there plans to keep this PR up to date and merge it? I can open a new one if the original author isn't available anymore @davisjam<https://github.com/davisjam>
—
Reply to this email directly, view it on GitHub<#29 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/AFOD3L6MIRGV5Q23DJKWPMTWE4A3RANCNFSM4I4RFHHA>.
You are receiving this because you were mentioned.Message ID: ***@***.***>
|
|
It looks like this PR has conflicts and the author hasn't been too active. I'll try to open a new one and make sure that it's up to date! |
|
Thank you
Get Outlook for iOS<https://aka.ms/o0ukef>
…________________________________
From: Rodry ***@***.***>
Sent: Tuesday, November 1, 2022 11:14:03 AM
To: davisjam/safe-regex ***@***.***>
Cc: Davis, James C ***@***.***>; Mention ***@***.***>
Subject: Re: [davisjam/safe-regex] added ts definitions (#29)
---- External Email: Use caution with attachments, links, or sharing data ----
It looks like this PR has conflicts and the author hasn't been too active. I'll try to open a new one and make sure that it's up to date!
—
Reply to this email directly, view it on GitHub<#29 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/AFOD3L3F7MIKEDGESLN3JCLWGEXTXANCNFSM4I4RFHHA>.
You are receiving this because you were mentioned.Message ID: ***@***.***>
|
|
Opened the new, up-to-date, PR at #52 |
feel free to contribute and keep it improving!