Skip to content

fix(types): add ResultSet interface and improve isResultSet typing#3481

Merged
josdejong merged 5 commits into
josdejong:developfrom
ranidam:feat/export-resultset-type
May 30, 2025
Merged

fix(types): add ResultSet interface and improve isResultSet typing#3481
josdejong merged 5 commits into
josdejong:developfrom
ranidam:feat/export-resultset-type

Conversation

@ranidam
Copy link
Copy Markdown
Contributor

@ranidam ranidam commented May 26, 2025

This PR introduces proper TypeScript support for the ResultSet returned by evaluate() when executing multiple expressions in MathJS.

Changes

  1. Added a ResultSet interface to types/index.d.ts
  2. Updated the isResultSet type guard to use a type predicate (x is ResultSet)
  3. Included a type-level test in test/typescript-tests/testTypes.ts to validate inference

When using evaluate() with multiple expressions (e.g.,1 + 1; 2 + 2;), MathJS returns a ResultSet.
However:
a) The ResultSet type was not previously exported
b) isResultSet() did not indicate that the input is a ResultSet
c) This caused issues when trying to type and handle evaluate() results safely

This PR fixes those gaps, allowing TypeScript developers to detect and type ResultSet results reliably.

Let me know if you'd like me to adjust anything further!

josdejong
josdejong requested changes May 28, 2025
View reviewed changes
Copy link
Copy Markdown
Owner

@josdejong josdejong left a comment

Choose a reason for hiding this comment

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

Thanks @ranidam for working out this PR!

I made three inline comments, can you have a look at those?

And can you fix the linting errors? You can automatically fix them by running npm run format, and you can check if there are linting issues by running npm run lint.

Comment thread types/index.d.ts Outdated
Comment thread types/index.d.ts Outdated
Comment thread test/typescript-tests/testTypes.ts Outdated
@ranidam
Copy link
Copy Markdown
Contributor Author

ranidam commented May 28, 2025

I've applied all requested changes: removed the [key: string]: any;, added toJSON(), cleaned up the unused variable, and ran npm run format. Moved the evaluate and isResultSet imports to the existing mathjs. Let me know if anything else is needed."

@ranidam
Copy link
Copy Markdown
Contributor Author

ranidam commented May 29, 2025

Thanks @josdejong for the clarification. I’ve updated toJSON() to return MathJSON and pushed the changes.

@josdejong
Copy link
Copy Markdown
Owner

josdejong commented May 30, 2025

Thanks Rani, all good to go now 👌

@josdejong josdejong merged commit 823d13e into josdejong:develop May 30, 2025
8 checks passed
@josdejong
Copy link
Copy Markdown
Owner

josdejong commented May 30, 2025

Your fix is published now in v14.5.2, thanks again!

@ranidam
Copy link
Copy Markdown
Contributor Author

ranidam commented May 30, 2025

Thanks so much, @josdejong! 🙌
Glad I could contribute.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@josdejong josdejong josdejong approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@ranidam @josdejong