From 9921e72527239e0261094258dd69ae1d1a9ad269 Mon Sep 17 00:00:00 2001 From: Chris Krycho Date: Mon, 26 Sep 2022 15:50:32 -0600 Subject: [PATCH] Fix internal issues caught by TS 4.9 TS 4.9 understands the `in` operator and catches two issues for us which earlier versions did not: 1. We were checking `name in model` in `Route#serialize` without checking that `model` there is an object. Given that a route's model is *not* guaranteed to be an object, this was a runtime error waiting to happen. `'a' in 2` will produce `TypeError: 2 is not an Object.' 2. We were checking for `Symbol.iterator in Array` in an internal `iterate()` utility in the `@ember/debug/data-adapter` package. This check is *always* true when targeting ES6 or newer for arrays, which would *appear* to makie the `else` branch a no-op on all supported browsers. Unfortunately, at least some consumers of this API implement a narrower contract for iterables (presumably due to legacy needs to interop with IE11 in Ember < 4.x). In those cases, we really have an iterable, *not* an array. --- packages/@ember/debug/data-adapter.ts | 17 ++++++++++++++++- packages/@ember/routing/route.ts | 2 +- 2 files changed, 17 insertions(+), 2 deletions(-) diff --git a/packages/@ember/debug/data-adapter.ts b/packages/@ember/debug/data-adapter.ts index bc3e4c8a7a1..a7cf8ace322 100644 --- a/packages/@ember/debug/data-adapter.ts +++ b/packages/@ember/debug/data-adapter.ts @@ -10,6 +10,7 @@ import { A as emberA } from '@ember/array'; import type { Cache } from '@glimmer/validator'; import { consumeTag, createCache, getValue, tagFor, untrack } from '@glimmer/validator'; import type ContainerDebugAdapter from '@ember/debug/container-debug-adapter'; +import { assert } from '.'; /** @module @ember/debug/data-adapter @@ -34,13 +35,27 @@ type WrappedRecord = { type RecordCallback = (records: Array<{ columnValues: object; object: T }>) => void; +// Represents the base contract for iterables as understood in the GLimmer VM +// historically. This is *not* the public API for it, because there *is* no +// public API for it. Recent versions of Glimmer simply use `Symbol.iterator`, +// but some older consumers still use this basic shape. +interface GlimmerIterable { + length: number; + forEach(fn: (value: T) => void): void; +} + function iterate(arr: Array, fn: (value: T) => void): void { if (Symbol.iterator in arr) { for (let item of arr) { fn(item); } } else { - arr.forEach(fn); + // SAFETY: this cast required to work this way to interop between TS 4.8 + // and 4.9. When we drop support for 4.8, it will narrow correctly via the + // use of the `in` operator above. (Preferably we will solve this by just + // switching to require `Symbol.iterator` instead.) + assert('', typeof (arr as unknown as GlimmerIterable).forEach === 'function'); + (arr as unknown as GlimmerIterable).forEach(fn); } } diff --git a/packages/@ember/routing/route.ts b/packages/@ember/routing/route.ts index 1e22eebda1d..9e45c5dec7f 100644 --- a/packages/@ember/routing/route.ts +++ b/packages/@ember/routing/route.ts @@ -353,7 +353,7 @@ class Route if (params.length === 1) { let [name] = params; assert('has name', name); - if (name in model) { + if (typeof model === 'object' && name in model) { object[name] = get(model, name); } else if (/_id$/.test(name)) { object[name] = get(model, 'id');