isEqual throws on Temporal values because it calls valueOf

Abierto Apto para principiantes
  • ¿Algo no está bien?
#139 0 comentarios 0 reacciones 0 asignados Ver en GitHub

Nadie ha tomado este issue todavía.

Evaluación

Dificultad
2/5
Tiempo estimado
1-3 horas
Aptitud para principiantes
82/100
Tipo de issue
Error
Claridad
Bien especificado
Estado de actividad
Activo
Stack tecnológico
javascript, node.js
Área
frontend, testing-qa

Línea de trabajo

Empieza en index.js alrededor de la línea 80, donde isEqual compara valores usando valueOf() antes de toString(). Reproduce el problema con los ejemplos de Temporal.PlainTime del issue, incluidos los valores anidados, y comprueba el comportamiento existente de Date y RegExp. El trabajo estará terminado cuando valores Temporal distintos pero iguales se comparen como true, los valores diferentes como false y las comparaciones anidadas devuelvan booleanos, sin cambiar el comportamiento de Date ni RegExp.

Escrito por el modelo de indexación a partir del texto del issue.

Descripción

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()`.
Lenguaje dominante
JavaScript
Estrellas
1.7k
Forks
54
Sin PR fusionados en 30 d

Preparar el entorno

Primeros pasos

  1. Lee el issue completo y luego la guía de contribución del proyecto.
  2. Comenta en el issue que vas a ocuparte — evita que dos personas hagan lo mismo.
  3. Haz un fork del repositorio y trabaja en una rama.
  4. Abre un pull request que haga referencia al número del issue.

Más de FormidableLabs/react-fast-compare

Todos los issues de FormidableLabs/react-fast-compare

Issues similares

Más issues de JavaScript

Recibe los nuevos issues en tu correo

Un resumen breve de issues de GitHub para principiantes.