Skip to content

Commit 9d1ef54

Browse files
Sander Toonenclaude
andcommitted
Fix ?? so non-numeric strings no longer trigger the fallback (0.6.2)
The old coalesce used global `isNaN(a)`, which coerces its argument to a number first — so `"hello" ?? "fallback"` (and any non-empty array or plain object) wrongly fell through to the fallback. Switch to `Number.isNaN`, guard the numeric branch with `typeof a === 'number'`, and treat empty string / array / object as missing values too. Also covers `-Infinity` for symmetry with `Infinity`. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 8ca5594 commit 9d1ef54

5 files changed

Lines changed: 53 additions & 19 deletions

File tree

docs/syntax.md

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -560,11 +560,15 @@ x == undefined → true (if x is not defined)
560560
561561
## Coalesce Operator
562562

563-
The `??` operator returns the right operand when the left operand is null, undefined, Infinity, or NaN:
563+
The `??` operator returns the right operand when the left operand is "missing" — that is,
564+
`null`, `undefined`, an empty string `""`, an empty array `[]`, an empty object `{}`,
565+
`Infinity` (positive or negative), or `NaN`:
564566

565567
```
566568
x ?? 0 → 0 (if x is undefined or null)
567569
y ?? "default" → y (if y has a value)
570+
"" ?? "default" → "default"
571+
[] ?? "default" → "default"
568572
10 / 0 ?? -1 → -1 (division by zero gives Infinity)
569573
sqrt(-1) ?? 0 → 0 (sqrt of negative gives NaN)
570574
```

packages/expreszo/package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
{
22
"name": "@pro-fa/expreszo",
3-
"version": "0.6.1",
3+
"version": "0.6.2",
44
"description": "Mathematical expression evaluator",
55
"keywords": [
66
"expression",

packages/expreszo/src/operators/binary/utility.ts

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
*/
55

66
import { AccessError } from '../../types/errors.js';
7+
import { isValueArray, isValueObject } from '../../types/type-guards.js';
78
import { DANGEROUS_PROPERTIES } from '../../validation/constants.js';
89

910
export function concat(a: any, b: any): any[] | string | undefined {
@@ -68,7 +69,12 @@ export function arrayIndexOrProperty(parent: any, index: number | string | undef
6869
}
6970

7071
export function coalesce(a: any, b: any): any {
71-
return a === undefined || a === null || a === Infinity || isNaN(a) ? b : a;
72+
if (a === undefined || a === null) return b;
73+
if (a === '') return b;
74+
if (typeof a === 'number' && (a === Infinity || a === -Infinity || Number.isNaN(a))) return b;
75+
if (isValueArray(a) && a.length === 0) return b;
76+
if (isValueObject(a) && Object.keys(a).length === 0) return b;
77+
return a;
7278
}
7379

7480
export function asOperator(a: any, b: string | undefined): any {

packages/expreszo/test/expression/expression-advanced.ts

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -400,6 +400,24 @@ describe('Expression Advanced Features TypeScript Test', () => {
400400
expect(parser.evaluate('NaN ?? 0')).toBe(0);
401401
});
402402

403+
it('returns the value for non-empty strings (regression: was returning fallback)', () => {
404+
expect(parser.evaluate('s ?? "fallback"', { s: 'hello' })).toBe('hello');
405+
});
406+
407+
it('returns the fallback for empty strings', () => {
408+
expect(parser.evaluate('s ?? "fallback"', { s: '' })).toBe('fallback');
409+
});
410+
411+
it('returns the fallback for empty arrays and objects', () => {
412+
expect(parser.evaluate('a ?? "fallback"', { a: [] })).toBe('fallback');
413+
expect(parser.evaluate('o ?? "fallback"', { o: {} })).toBe('fallback');
414+
});
415+
416+
it('passes through non-empty arrays and objects', () => {
417+
expect(parser.evaluate('a ?? "fallback"', { a: [1, 2] })).toEqual([1, 2]);
418+
expect(parser.evaluate('o ?? "fallback"', { o: { x: 1 } })).toEqual({ x: 1 });
419+
});
420+
403421
it('should be disabled by the coalesce option', () => {
404422
const parser = new Parser({ operators: { coalesce: false } });
405423
expect(() => parser.evaluate('1 ?? 2')).toThrow(/Unknown character/);

packages/expreszo/test/operators/binary/utility.ts

Lines changed: 22 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -308,28 +308,39 @@ describe('Binary Utility Operators', () => {
308308
expect(coalesce(Infinity, 'fallback')).toBe('fallback');
309309
});
310310

311+
it('should return fallback when first value is -Infinity', () => {
312+
expect(coalesce(-Infinity, 'fallback')).toBe('fallback');
313+
});
314+
311315
it('should return fallback when first value is NaN', () => {
312316
expect(coalesce(NaN, 'fallback')).toBe('fallback');
313317
});
314318

315-
it('should return 0 when first value is 0', () => {
316-
expect(coalesce(0, 'fallback')).toBe(0);
319+
it('should return fallback when first value is empty string', () => {
320+
expect(coalesce('', 'fallback')).toBe('fallback');
321+
});
322+
323+
it('should return fallback when first value is empty array', () => {
324+
expect(coalesce([], 'fallback')).toBe('fallback');
317325
});
318326

319-
it('should return empty string when first value is empty string', () => {
320-
expect(coalesce('', 'fallback')).toBe('');
327+
it('should return fallback when first value is empty object', () => {
328+
expect(coalesce({}, 'fallback')).toBe('fallback');
329+
});
330+
331+
it('should return 0 when first value is 0', () => {
332+
expect(coalesce(0, 'fallback')).toBe(0);
321333
});
322334

323335
it('should return false when first value is false', () => {
324336
expect(coalesce(false, 'fallback')).toBe(false);
325337
});
326338

327-
it('should return fallback for non-numeric strings (isNaN coerces to NaN)', () => {
328-
expect(coalesce('hello', 'fallback')).toBe('fallback');
339+
it('should return non-numeric strings unchanged', () => {
340+
expect(coalesce('hello', 'fallback')).toBe('hello');
329341
});
330342

331343
it('should return numeric string when it is valid', () => {
332-
// isNaN('42') is false because '42' coerces to 42
333344
expect(coalesce('42', 'fallback')).toBe('42');
334345
});
335346

@@ -338,17 +349,12 @@ describe('Binary Utility Operators', () => {
338349
expect(coalesce(-1, 0)).toBe(-1);
339350
});
340351

341-
it('should return fallback for arrays (isNaN coerces array to NaN)', () => {
342-
expect(coalesce([1, 2], 'fallback')).toBe('fallback');
343-
});
344-
345-
it('should return fallback for objects (isNaN coerces object to NaN)', () => {
346-
expect(coalesce({ a: 1 }, 'fallback')).toBe('fallback');
352+
it('should return non-empty arrays unchanged', () => {
353+
expect(coalesce([1, 2], 'fallback')).toEqual([1, 2]);
347354
});
348355

349-
it('should return -Infinity (not caught by === Infinity or isNaN)', () => {
350-
// -Infinity !== Infinity, and isNaN(-Infinity) is false
351-
expect(coalesce(-Infinity, 'fallback')).toBe(-Infinity);
356+
it('should return non-empty objects unchanged', () => {
357+
expect(coalesce({ a: 1 }, 'fallback')).toEqual({ a: 1 });
352358
});
353359
});
354360

0 commit comments

Comments
 (0)