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

feat: copied d.ts types from DefinitelyTyped #3846

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

kelvinsjk
Copy link

What is the previous behavior before this PR?

Using KaTeX in TypeScript meant having to install the types from @types/katex

What is the new behavior after this PR?

The types are now bundled together

Closes #3531

Notes: I couldn't get the types to work correctly with the "exports" section in the package.json file, so I didn't include the types for the auto-render extension that are on DefinitelyTyped. However, have tested locally that the "types" key in the package.json file works as intended and provides types to my TypeScript imports of the main KaTeX library

@joshuawwright
Copy link

@edemaine Any chance this can be approved?

Copy link
Member

@edemaine edemaine left a comment

Choose a reason for hiding this comment

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

Sorry for the long delay. Have you tested this in both ESM and CJS modes? I intend to but it would be good for both of us to check. I thought getting both to work would require more effort than what I see here.

/**
* Limit the number of macro expansions to specified number
*
* If set to `Infinity`, marco expander will try to fully expand
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
* If set to `Infinity`, marco expander will try to fully expand
* If set to `Infinity`, macro expander will try to fully expand

maxExpand?: number | undefined;
/**
* If `false` or `"ignore"`, allow features that make
* writing in LaTex convenient but not supported by LaTex
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
* writing in LaTex convenient but not supported by LaTex
* writing in LaTeX convenient but not supported by LaTeX

This clear typo makes me wonder whether we should instead copy descriptions from the official documentation, which are likely more precise.

@kelvinsjk
Copy link
Author

I only tested naively locally (letting typescript handle the import). Will try out for both esm and cjs.

Will also give the extensions typing another go and use the official documentation instead of the blind copy/paste from the types package currently.

I foresee taking a couple of weeks to get back to this, so if anyone wants to give it a go before that please do!

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.

Distribute typescript definitions
3 participants