isEqual throws on Temporal values because it calls valueOf

Aperta Adatta ai principianti
#139 0 commenti 0 reazioni 0 assegnatari Vedi su GitHub

Nessuno ha ancora preso questa issue.

Valutazione

Difficoltà
2/5
Tempo stimato
1-3 ore
Idoneità per principianti
82/100
Tipo di issue
Bug
Chiarezza
Specificata chiaramente
Stato di attività
Attiva
Stack tecnologico
javascript, node.js

Direzione di ricerca

Inizia in index.js intorno alla riga 80, dove isEqual confronta i valori usando valueOf() prima di toString(). Riproduci il problema con gli esempi Temporal.PlainTime dell’issue, compresi i valori annidati, e controlla il comportamento esistente di Date e RegExp. Il lavoro è completo quando valori Temporal distinti ma uguali vengono confrontati come true, valori diversi come false e i confronti annidati restituiscono booleani, senza cambiare il comportamento di Date o RegExp.

Scritto dal modello di indicizzazione a partire dal testo della issue.

Descrizione

bug
Is there an existing issue for this?
  • I have searched the existing issues
Code of Conduct
  • I agree to follow this project's Code of Conduct
Code Sandbox link

No response

Bug report
## Description

`isEqual` throws when comparing two distinct Temporal values (or objects that contain them). `Temporal.*.prototype.valueOf` is specified to throw a `TypeError` so that `<` and `>` cannot be used by accident. Equality is supposed to go through `equals` or `compare`.

`react-fast-compare` 3.2.2 calls `valueOf` whenever an object overrides it, before the `toString` fallback:


if (a.valueOf !== Object.prototype.valueOf && typeof a.valueOf === 'function' && typeof b.valueOf === 'function') return a.valueOf() === b.valueOf();
if (a.toString !== Object.prototype.toString && typeof a.toString === 'function' && typeof b.toString === 'function') return a.toString() === b.toString();


`index.js`, around line 80. The `toString` line is never reached. Temporal’s `toString()` returns a stable ISO string and does not throw, so that fallback would compare these values correctly. `Date` can keep using `valueOf()`, which returns a number and does not throw.

The same reference still compares equal, because of the `a === b` check at the start. The throw only happens for two different instances, including two instances of the same time.

## Reproduction

Node 26, with Temporal built in. `[email protected]`.


const isEqual = require("react-fast-compare");

const a = Temporal.PlainTime.from("07:00");
const b = Temporal.PlainTime.from("07:00");

isEqual(a, a); // true
isEqual(a, b); // TypeError
isEqual({ time: a }, { time: b }); // TypeError


Actual message:


TypeError: Do not use Temporal.PlainTime.prototype.valueOf; use Temporal.PlainTime.prototype.compare for comparison.


The same throw happens for `PlainDate`, `PlainDateTime`, `Instant`, `ZonedDateTime`, and `Duration`.

## Expected

`isEqual` returns a boolean.

- `isEqual(a, b)` is `true` when the two times represent the same time.
- `isEqual(a, Temporal.PlainTime.from("08:00"))` is `false`.
- Nested values behave the same way.
- `Date` and `RegExp` keep their current behavior.

## Context

`@carbon/react` 1.118 depends on `react-fast-compare@^3.2.2`. Its `DataTable` deep-compares row cell values with `isEqual`. A cell that holds a domain object containing a `Temporal.PlainTime` crashes the table when those instances are not the same reference. Carbon’s ComboBox and multi-select use the same comparison.

## Suggested fix

If `valueOf()` throws, fall through to the existing `toString()` comparison instead of failing the whole `isEqual` call. That keeps `Date` on `valueOf()` and lets Temporal use its ISO `toString()`.
Lingua principale
JavaScript
Stelle
1.7k
Fork
54
Metriche di merge delle PR
Nessuna PR unita negli ultimi 30g

Preparare l'ambiente

Come iniziare

  1. Leggi tutta la issue e poi la guida ai contributi del progetto.
  2. Commenta sulla issue per dire che te ne occupi tu — evita che due persone facciano lo stesso lavoro.
  3. Fai un fork del repository e lavora su un branch.
  4. Apri una pull request che faccia riferimento al numero della issue.

Altre issue di FormidableLabs/react-fast-compare

Tutte le issue di FormidableLabs/react-fast-compare

Issue simili

Altre issue su JavaScript

Ricevi le nuove issue nella tua casella

Un breve riepilogo di issue GitHub adatte ai principianti.