Skip to content

Support for optional chaining#3547

Merged
josdejong merged 17 commits intojosdejong:developfrom
NilsDietrich:feat/optional-chaining
Oct 29, 2025
Merged

Support for optional chaining#3547
josdejong merged 17 commits intojosdejong:developfrom
NilsDietrich:feat/optional-chaining

Conversation

@NilsDietrich
Copy link
Copy Markdown
Contributor

This pull request adds support for optional chaining '.?' in the expression parser. Till now it's hard to securely access paths of objects which may be undefined:

Scope: { a: { b: { c: 3 } } }
Expression: 'result = (a==undefined ? undefined : (a.b==undefined ? undefined: a.b.c ))'

With optional chaining this can be simplified to:
'result = a?.b?.c'

This feature helps to make the expression language more handy (after support for the nullish coalescing operator (??) was added in #3497).

The new functionalty is fully provided by changes to the AccessorNode.

@josdejong
Copy link
Copy Markdown
Owner

Awesome Nils, I love optional chaining. I'll review your PR coming week.

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.

Awesome job Nils, this is a great addition! The code is very readable and the comments here and there are helpful. Thanks for working out the documentation and tests.

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

@josdejong
Copy link
Copy Markdown
Owner

I see some tests are failing on Firefox. Can you have a look at that?

See https://github.com/josdejong/mathjs/actions/runs/18316821660/job/52338944661

@NilsDietrich
Copy link
Copy Markdown
Contributor Author

I will have a look for the given points at the weekend or next week. I don't like the duplications of the eval funtions either, but I wanted to achieve the best possible eval-performance and don't do any comparisons there, which can be done at compile time. But I will look if there is a better solution and will run some benchmark to check the impact .

@josdejong
Copy link
Copy Markdown
Owner

Thanks!

@josdejong
Copy link
Copy Markdown
Owner

@NilsDietrich did you manage to have a look at the last open ends? I'm looking forward to have this PR merged :). Please let me know if you need help.

@NilsDietrich
Copy link
Copy Markdown
Contributor Author

I fixed the things you mentioned. I've also added a benchmark for the AccessorNode. I was unable to measure any performance benefits from having different callback-variants, so I simplified the code to only have the original callbacks and check the optional-chaining paths inside.

@josdejong
Copy link
Copy Markdown
Owner

Thanks Nils! That sounds good. I'll review your PR next week (time for weekend now 😅).

@josdejong
Copy link
Copy Markdown
Owner

Thanks @NilsDietrich , all looks good! I'm glad you where able to simplify the code in AccessorNode without performance penalty.

@josdejong josdejong merged commit 48ba6ad into josdejong:develop Oct 29, 2025
8 checks passed
@josdejong
Copy link
Copy Markdown
Owner

Published now in v15.1.0, thanks again!

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.

2 participants