Skip to content

Commit

Permalink
Fix internal issues caught by TS 4.9
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
chriskrycho committed Sep 27, 2022
1 parent 5bb4934 commit 9921e72
Show file tree
Hide file tree
Showing 2 changed files with 17 additions and 2 deletions.
17 changes: 16 additions & 1 deletion packages/@ember/debug/data-adapter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -34,13 +35,27 @@ type WrappedRecord<T> = {

type RecordCallback<T> = (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<T> {
length: number;
forEach(fn: (value: T) => void): void;
}

function iterate<T>(arr: Array<T>, 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<T>).forEach === 'function');
(arr as unknown as GlimmerIterable<T>).forEach(fn);
}
}

Expand Down
2 changes: 1 addition & 1 deletion packages/@ember/routing/route.ts
Original file line number Diff line number Diff line change
Expand Up @@ -353,7 +353,7 @@ class Route<T = unknown>
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');
Expand Down

0 comments on commit 9921e72

Please sign in to comment.