Fix IFS short-circuit evaluation#1676
Open
JfanLiu wants to merge 2 commits into
Open
Conversation
✅ Deploy Preview for hyperformula-docs ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
✅ Deploy Preview for hyperformula-dev-docs ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
70cc0ba to
c8987fa
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Context
IFSshould stop evaluating condition/value pairs once it finds the first true condition. This matches Microsoft Excel and Google Sheets behavior.For example, Excel returns
1for:=IFS(TRUE(), 1, 1/0, 2)HyperFormula previously evaluated later arguments and returned an error.
How did you test your changes?
=IFS(TRUE(), 1, 1/0, 2).npm run test:jest -- smoke.npm run verify:typings.npm run lint.Related issues:
N/A
Checklist:
Note
Low Risk
Low risk: changes are localized to
IFSevaluation logic and add a regression test; main risk is subtle behavior differences in array arithmetic/vectorized cases.Overview
Fixes
IFSto short-circuit evaluation: condition/value pairs are now evaluated sequentially and stop once the first true condition is found, avoiding errors from later branches (e.g.1/0).Keeps the prior vectorized implementation for array arithmetic by falling back to
runFunctionwhen array/range arguments are detected, and adds a smoke regression test plus a CHANGELOG entry documenting the fix.Reviewed by Cursor Bugbot for commit c8987fa. Bugbot is set up for automated code reviews on this repo. Configure here.