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

GH-284: Add TypeScript definitions and tests #285

Merged
merged 1 commit into from Feb 24, 2020

Conversation

jfahrenkrug
Copy link
Contributor

Ported TS definitions from DefinitelyTyped, including tests.

Once a version with these definitions is released, we will need
to remove the definitions from DefinitelyTyped:
https://github.com/DefinitelyTyped/DefinitelyTyped#removing-a-package

Ported TS definitions from DefinitelyTyped, including tests.

Once a version with these definitions is released, we will need
to remove the definitions from DefinitelyTyped:
https://github.com/DefinitelyTyped/DefinitelyTyped#removing-a-package
Copy link
Collaborator

@josephfrazier josephfrazier left a comment

Choose a reason for hiding this comment

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

I'm no TS expert, but I'd be glad to include definitions in the package! Are you sure they're up to date? I'm just asking because I can't easily verify it myself, and it looks like DefinitelyTyped only has types up to version 3.0.0: https://github.com/DefinitelyTyped/DefinitelyTyped/blob/f2db20ef615bd88a605a9b8750ec8ba170e2415a/types/xregexp/index.d.ts

@jfahrenkrug
Copy link
Contributor Author

@josephfrazier Good catch! I didn't do a simple port though. I used https://github.com/Microsoft/dts-gen to create a baseline, then manually checked the JS methods to make sure the types are correct (dts-gen uses any a lot). Please let me know if you want to have any improvements to the code!

Copy link
Collaborator

@josephfrazier josephfrazier left a comment

Choose a reason for hiding this comment

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

Cool, thanks for the explanation, dts-gen and dtslint sound pretty neat, at a glance!

@josephfrazier josephfrazier merged commit 8490516 into slevithan:master Feb 24, 2020
@slevithan
Copy link
Owner

@jfahrenkrug This may be a lot to ask, but any ideas why dtslint v3.0.0+ (from 2 years ago, released a few days after this pull request) is failing for XRegExp? New versions of dtslint fail with error "TypeError: Cannot read property 'NameOnly' of undefined", but there are no references to "NameOnly" anywhere in the XRegExp codebase and I haven't been able to find information about dtslint v3 breaking changes.

@jfahrenkrug
Copy link
Contributor Author

@slevithan Hey Steven, I have no idea. I'd be happy to take a look, but I'm not sure if I'll find time this week.

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.

None yet

3 participants