Skip to content

Reduce size impact of Target/TargetImpl pattern #3272

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 11 commits into from
Jun 26, 2020
Merged
2 changes: 2 additions & 0 deletions .changeset/tricky-rocks-dream.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
---
---
39 changes: 18 additions & 21 deletions packages/firestore/src/core/target.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,31 +37,28 @@ import { debugCast } from '../util/assert';
* maps to a single WatchTarget in RemoteStore and a single TargetData entry
* in persistence.
*/
export class Target {
protected constructor(
readonly path: ResourcePath,
readonly collectionGroup: string | null,
readonly orderBy: OrderBy[],
readonly filters: Filter[],
readonly limit: number | null,
readonly startAt: Bound | null,
readonly endAt: Bound | null
) {}
export interface Target {
readonly path: ResourcePath;
readonly collectionGroup: string | null;
readonly orderBy: OrderBy[];
readonly filters: Filter[];
readonly limit: number | null;
readonly startAt: Bound | null;
readonly endAt: Bound | null;
}

class TargetImpl extends Target {
// Visible for testing
export class TargetImpl implements Target {
memoizedCanonicalId: string | null = null;
constructor(
path: ResourcePath,
collectionGroup: string | null = null,
orderBy: OrderBy[] = [],
filters: Filter[] = [],
limit: number | null = null,
startAt: Bound | null = null,
endAt: Bound | null = null
) {
super(path, collectionGroup, orderBy, filters, limit, startAt, endAt);
}
readonly path: ResourcePath,
readonly collectionGroup: string | null = null,
readonly orderBy: OrderBy[] = [],
readonly filters: Filter[] = [],
readonly limit: number | null = null,
readonly startAt: Bound | null = null,
readonly endAt: Bound | null = null
) {}
}

/**
Expand Down
9 changes: 7 additions & 2 deletions packages/firestore/test/unit/local/target_cache.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,12 @@ import {
import { Timestamp } from '../../../src/api/timestamp';
import * as persistenceHelpers from './persistence_test_helpers';
import { TestTargetCache } from './test_target_cache';
import { canonifyTarget, Target, targetEquals } from '../../../src/core/target';
import {
canonifyTarget,
Target,
targetEquals,
TargetImpl
} from '../../../src/core/target';

describe('MemoryTargetCache', () => {
genericTargetCacheTests(persistenceHelpers.testMemoryEagerPersistence);
Expand Down Expand Up @@ -106,7 +111,7 @@ describe('IndexedDbTargetCache', () => {
function genericTargetCacheTests(
persistencePromise: () => Promise<Persistence>
): void {
addEqualityMatcher({ equalsFn: targetEquals, forType: Target });
addEqualityMatcher({ equalsFn: targetEquals, forType: TargetImpl });
let cache: TestTargetCache;

const QUERY_ROOMS = Query.atPath(path('rooms')).toTarget();
Expand Down
4 changes: 2 additions & 2 deletions packages/firestore/test/unit/specs/describe_spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ import { SpecBuilder } from './spec_builder';
import { SpecStep } from './spec_test_runner';

import * as stringify from 'json-stable-stringify';
import { Target, targetEquals } from '../../../src/core/target';
import { targetEquals, TargetImpl } from '../../../src/core/target';

// Disables all other tests; useful for debugging. Multiple tests can have
// this tag and they'll all be run (but all others won't).
Expand Down Expand Up @@ -240,7 +240,7 @@ export function describeSpec(

if (!writeJSONFile) {
describe(name, () => {
addEqualityMatcher({ equalsFn: targetEquals, forType: Target });
addEqualityMatcher({ equalsFn: targetEquals, forType: TargetImpl });
return builder();
});
} else {
Expand Down