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

Fix the simplifyCore fix #2473

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

Conversation

joshhansen
Copy link
Contributor

In #2456 I failed to introduce simplifyCore typing to MathJsStatic (it was only added to MathJsChain). This PR fixes that omission. It also changes the return type of the MathJsChain form of simplifyCore to be MathJsChain which is the convention in that environment.

In josdejong#2456 I failed to introduce `simplifyCore` typing to `MathJsStatic` (it was only added to `MathJsChain`). This PR fixes that omission. It also changes the return type of the `MathJsChain` form of `simplifyCore` to be `MathJsChain` which is the convention in that environment.
@josdejong
Copy link
Owner

Thanks Josh 👍

I think the definition of the chain method must be:

simplifyCore(): MathJsChain;

since the expr is already in the chain object. Usage is like:

const expr = '2 + 3'
math.chain(expr)
  .parse()
  .simplifyCore()
  .done()
  .toString() // 5

@gwhitney
Copy link
Collaborator

gwhitney commented Mar 8, 2022

And now that we have testing of the TypeScript typing by way of running types/index.ts, please could you add code to the latter file which will verify that math.simplifyCore is correctly typed? Thanks!

@gwhitney
Copy link
Collaborator

I am willing to make the small tweaks it would take to get this mergeable.

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