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, making the
   `else` branch a no-op on all currently-supported browsers.
  • Loading branch information
chriskrycho committed Sep 26, 2022
1 parent 5551dde commit dda3ffc
Show file tree
Hide file tree
Showing 2 changed files with 7 additions and 16 deletions.
21 changes: 6 additions & 15 deletions packages/@ember/debug/data-adapter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,16 +34,6 @@ type WrappedRecord<T> = {

type RecordCallback<T> = (records: Array<{ columnValues: object; object: T }>) => 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);
}
}

class RecordsWatcher<T> {
recordCaches: Map<T, Cache<void>> = new Map();

Expand Down Expand Up @@ -88,10 +78,10 @@ class RecordsWatcher<T> {
// Track `[]` for legacy support
consumeTag(tagFor(records, '[]'));

iterate(records, (record) => {
for (let record of records) {
getValue(this.getCacheForItem(record));
seen.add(record);
});
}

// Untrack this operation because these records are being removed, they
// should not be polled again in the future
Expand Down Expand Up @@ -133,9 +123,10 @@ class TypeWatcher {
let hasBeenAccessed = false;

this.cache = createCache(() => {
// Empty iteration, we're doing this just
// to track changes to the records array
iterate(records, () => {});
for (let _ of records) {
// Empty iteration, we're doing this just
// to track changes to the records array
}

// Also track `[]` for legacy support
consumeTag(tagFor(records, '[]'));
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 dda3ffc

Please sign in to comment.