Skip to content

Commit

Permalink
type: enable exactOptionalPropertyTypes option (tensorflow#5459)
Browse files Browse the repository at this point in the history
Before this change, below held true:
```ts
interface Foo {
  bar?: number;
}

// Legal
const foo1: Foo = {};
// Legal
const foo2: Foo = {bar: undefined};
```

After this change,
```ts
interface Foo {
  bar?: number;
  baz: number|undefined;
}

// Illegal; `bar` cannot be `undefined` and must be number.
const foo1: Foo = {bar: undefined, baz: undefined};
// Illegal; `baz` property must exist. Must be one of `number` of
// `undefined`.
const foo2: Foo = {};
// Legal; `bar` property does not exist while `baz` is specified.
const foo3: Foo = {baz: undefined};
```

When a property is defined as `?:`, it does not mean that the property
can be `undefined` but it means a property can be omitted altogether or
it must exist with the value of the type as defined. As such, when a
property is written with `undefined`, the value type must explicitly
allow `undefined` like `baz` example above.
  • Loading branch information
stephanwlee authored and yatbear committed Mar 27, 2023
1 parent ed50b00 commit 6fb4cad
Show file tree
Hide file tree
Showing 26 changed files with 133 additions and 85 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -342,7 +342,7 @@ export const getFocusedGraphOpInputs = createSelector(
...inputSpec,
};
if (graph.has(inputSpec.op_name)) {
spec.data = graph.get(inputSpec.op_name);
spec.data = graph.get(inputSpec.op_name)!;
}
return spec;
});
Expand Down Expand Up @@ -427,7 +427,7 @@ export const getFocusedGraphOpConsumers = createSelector(
return slotConsumers.map((consumerSpec) => {
const spec: GraphOpConsumerSpec = {...consumerSpec};
if (graph.has(consumerSpec.op_name)) {
spec.data = graph.get(consumerSpec.op_name);
spec.data = graph.get(consumerSpec.op_name)!;
}
return spec;
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -129,20 +129,12 @@ describe('Graph Container', () => {
},
];
store.overrideSelector(getFocusedGraphOpInfo, op2);
store.overrideSelector(getFocusedGraphOpInputs, [
{
...op2.inputs[0],
data: neighborDataAvailable ? op1 : undefined,
},
]);
store.overrideSelector(getFocusedGraphOpConsumers, [
[
{
...op2.consumers[0][0],
data: neighborDataAvailable ? op3 : undefined,
},
],
]);
const input = {...op2.inputs[0]};
if (neighborDataAvailable) input.data = op1;
store.overrideSelector(getFocusedGraphOpInputs, [input]);
const consumer = {...op2.consumers[0][0]};
if (neighborDataAvailable) consumer.data = op3;
store.overrideSelector(getFocusedGraphOpConsumers, [[consumer]]);

fixture.detectChanges();

Expand Down
8 changes: 6 additions & 2 deletions tensorboard/webapp/alert/alert_action_test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,9 @@ describe('alert_effects', () => {
actionCreator: alertActionOccurred,
alertFromAction: (action: Action) => {
if (shouldReportAlert) {
return {localizedMessage: 'alert details'};
return {
localizedMessage: 'alert details',
};
}
return null;
},
Expand All @@ -67,7 +69,9 @@ describe('alert_effects', () => {
actions$.next(alertActionOccurred);

expect(recordedActions).toEqual([
alertActions.alertReported({localizedMessage: 'alert details'}),
alertActions.alertReported({
localizedMessage: 'alert details',
}),
]);
});

Expand Down
15 changes: 8 additions & 7 deletions tensorboard/webapp/alert/store/alert_reducers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ limitations under the License.
==============================================================================*/
import {Action, createReducer, on} from '@ngrx/store';
import * as actions from '../actions';
import {AlertInfo} from '../types';
import {AlertState} from './alert_types';

const initialState: AlertState = {
Expand All @@ -25,14 +26,14 @@ const reducer = createReducer(
on(
actions.alertReported,
(state: AlertState, {localizedMessage, followupAction}): AlertState => {
return {
...state,
latestAlert: {
localizedMessage,
followupAction,
created: Date.now(),
},
const latestAlert: AlertInfo = {
localizedMessage,
created: Date.now(),
};
if (followupAction) {
latestAlert.followupAction = followupAction;
}
return {...state, latestAlert};
}
)
);
Expand Down
1 change: 0 additions & 1 deletion tensorboard/webapp/alert/store/alert_reducers_test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,6 @@ describe('alert_reducers', () => {
const state2 = alertReducers.reducers(state1, action1);
expect(state2.latestAlert!).toEqual({
localizedMessage: 'Foo1 failed',
followupAction: undefined,
created: 123,
});

Expand Down
10 changes: 7 additions & 3 deletions tensorboard/webapp/app_routing/route_config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ interface PositiveMatch {
params: RouteParams;
pathParts: string[];
isRedirection: boolean;
redirectionQueryParams?: SerializableQueryParams;
redirectionQueryParams: SerializableQueryParams | undefined;
}

type Match = NegativeMatch | PositiveMatch;
Expand Down Expand Up @@ -172,6 +172,7 @@ abstract class RouteConfigMatcher {
params: combinedParams,
pathParts,
isRedirection: false,
redirectionQueryParams: undefined,
};
}

Expand All @@ -186,6 +187,7 @@ abstract class RouteConfigMatcher {
params,
pathParts,
isRedirection: false,
redirectionQueryParams: undefined,
};
}

Expand Down Expand Up @@ -243,6 +245,7 @@ class StaticRedirectionRouteConfigMatcher extends RouteConfigMatcher {
params: match.params,
pathParts: newPathParts,
isRedirection: true,
redirectionQueryParams: undefined,
};
}
}
Expand Down Expand Up @@ -285,7 +288,7 @@ export interface NonRedirectionRouteMatch extends BaseRedirectionRouteMatch {

export interface RedirectionRouteMatch extends BaseRedirectionRouteMatch {
originateFromRedirection: true;
redirectionOnlyQueryParams?: Route['queryParams'];
redirectionOnlyQueryParams: Route['queryParams'] | undefined;
}

export type RouteMatch = NonRedirectionRouteMatch | RedirectionRouteMatch;
Expand Down Expand Up @@ -441,7 +444,8 @@ export class RouteConfigs {
deepLinkProvider: definition.deepLinkProvider ?? null,
pathname: definition.path,
params: {},
originateFromRedirection: wasRedirected,
originateFromRedirection: true,
redirectionOnlyQueryParams: undefined,
};
}

Expand Down
9 changes: 8 additions & 1 deletion tensorboard/webapp/app_routing/route_config_test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ function buildRouteMatch(override: Partial<RouteMatch> = {}): RouteMatch {
deepLinkProvider: null,
originateFromRedirection: false,
...override,
};
} as RouteMatch;
}

describe('route config', () => {
Expand Down Expand Up @@ -224,6 +224,8 @@ describe('route config', () => {
routeKind: RouteKind.EXPERIMENT,
pathname: '/tb/bestest',
params: {},
originateFromRedirection: true,
redirectionOnlyQueryParams: undefined,
})
);
});
Expand Down Expand Up @@ -268,6 +270,8 @@ describe('route config', () => {
routeKind: RouteKind.EXPERIMENT,
pathname: '/tb',
params: {},
originateFromRedirection: true,
redirectionOnlyQueryParams: undefined,
})
);
});
Expand Down Expand Up @@ -494,6 +498,8 @@ describe('route config', () => {
routeKind: RouteKind.EXPERIMENT,
pathname: '/default',
params: {},
originateFromRedirection: true,
redirectionOnlyQueryParams: undefined,
})
);
});
Expand All @@ -508,6 +514,7 @@ describe('route config', () => {
buildRouteMatch({
pathname: '/c',
originateFromRedirection: true,
redirectionOnlyQueryParams: undefined,
})
);
});
Expand Down
3 changes: 2 additions & 1 deletion tensorboard/webapp/experiments/store/experiments_reducers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,10 @@ import {
createReducer,
} from '@ngrx/store';
import {DEFAULT_EXPERIMENT_ID} from '../../app_routing/types';
import {Experiment} from '../types';
import {ExperimentsDataState, ExperimentsState} from './experiments_types';

const defaultExperiment = {
const defaultExperiment: Experiment = {
id: DEFAULT_EXPERIMENT_ID,
name: '',
start_time: 0,
Expand Down
4 changes: 1 addition & 3 deletions tensorboard/webapp/experiments/store/testing.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,13 +24,11 @@ import {
* Builds an experiment from default. Can override fields by providing
* `override`.
*/
export function buildExperiment(override?: Partial<Experiment>) {
export function buildExperiment(override?: Partial<Experiment>): Experiment {
return {
id: '1',
description: undefined,
name: 'Default Experiment',
start_time: 1,
tags: undefined,
...override,
};
}
Expand Down
2 changes: 1 addition & 1 deletion tensorboard/webapp/metrics/actions/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -173,7 +173,7 @@ export const timeSelectionChanged = createAction(
'[Metrics] Linked Time Selection Changed',
props<{
startStep: number;
endStep?: number;
endStep: number | undefined;
}>()
);

Expand Down
10 changes: 7 additions & 3 deletions tensorboard/webapp/metrics/effects/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -199,9 +199,13 @@ export class MetricsEffects implements OnInitEffects {
*/
const requests: TimeSeriesRequest[] = fetchInfos.map((fetchInfo) => {
const {plugin, tag, runId, sample} = fetchInfo;
return isSingleRunPlugin(plugin)
? {plugin, tag, sample, runId: runId!}
: {plugin, tag, sample, experimentIds};
const partialRequest: TimeSeriesRequest = isSingleRunPlugin(plugin)
? {plugin, tag, runId: runId!}
: {plugin, tag, experimentIds};
if (sample !== undefined) {
partialRequest.sample = sample;
}
return partialRequest;
});

// Fetch and handle responses.
Expand Down
13 changes: 4 additions & 9 deletions tensorboard/webapp/metrics/effects/metrics_effects_test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ import {
PluginType,
SingleRunPluginType,
TagMetadata,
TimeSeriesRequest,
TimeSeriesResponse,
} from '../data_source';
import {getMetricsTagMetadataLoadState} from '../store';
Expand Down Expand Up @@ -328,11 +329,10 @@ describe('metrics effects', () => {
}
}

function buildTimeSeriesResponse() {
function buildTimeSeriesResponse(): TimeSeriesResponse {
return {
plugin: PluginType.SCALARS,
tag: 'tagA',
sample: undefined,
runToSeries: {
run1: createScalarStepData(),
},
Expand Down Expand Up @@ -377,13 +377,11 @@ describe('metrics effects', () => {
plugin: PluginType.SCALARS as MultiRunPluginType,
tag: 'tagA',
experimentIds: ['exp1'],
sample: undefined,
},
{
plugin: PluginType.SCALARS as MultiRunPluginType,
tag: 'tagA',
experimentIds: ['exp1'],
sample: undefined,
},
],
}),
Expand Down Expand Up @@ -425,7 +423,6 @@ describe('metrics effects', () => {
plugin: PluginType.SCALARS as MultiRunPluginType,
tag: 'tagA',
experimentIds: ['exp1'],
sample: undefined,
},
],
}),
Expand Down Expand Up @@ -594,11 +591,10 @@ describe('metrics effects', () => {
})
);

const expectedRequest = {
const expectedRequest: TimeSeriesRequest = {
plugin: PluginType.SCALARS as MultiRunPluginType,
tag: 'tagA',
experimentIds: ['exp1'],
sample: undefined,
};
expect(fetchTimeSeriesSpy.calls.count()).toBe(1);
expect(fetchTimeSeriesSpy).toHaveBeenCalledWith([expectedRequest]);
Expand Down Expand Up @@ -693,12 +689,11 @@ describe('metrics effects', () => {
})
);

const expectedRequests = [
const expectedRequests: TimeSeriesRequest[] = [
{
plugin: PluginType.SCALARS as MultiRunPluginType,
tag: 'tagA',
experimentIds: ['exp1'],
sample: undefined,
},
{
plugin: PluginType.IMAGES as SingleRunPluginType,
Expand Down
14 changes: 4 additions & 10 deletions tensorboard/webapp/metrics/store/metrics_reducers_test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2018,7 +2018,7 @@ describe('metrics reducers', () => {

const after = reducers(
before,
actions.timeSelectionChanged({startStep: 2})
actions.timeSelectionChanged({startStep: 2, endStep: undefined})
);

expect(after.selectedTime).toEqual({
Expand Down Expand Up @@ -2058,9 +2058,7 @@ describe('metrics reducers', () => {

const after = reducers(
before,
actions.timeSelectionChanged({
startStep: 2,
})
actions.timeSelectionChanged({startStep: 2, endStep: undefined})
);

expect(after.selectedTime).toEqual({
Expand All @@ -2077,9 +2075,7 @@ describe('metrics reducers', () => {

const nextState = reducers(
beforeState,
actions.timeSelectionChanged({
startStep: 2,
})
actions.timeSelectionChanged({startStep: 2, endStep: undefined})
);

expect(nextState.selectTimeEnabled).toBe(true);
Expand Down Expand Up @@ -2116,9 +2112,7 @@ describe('metrics reducers', () => {

const nextState = reducers(
beforeState,
actions.timeSelectionChanged({
startStep: 150,
})
actions.timeSelectionChanged({startStep: 150, endStep: undefined})
);

expect(nextState.selectedTime).toEqual({
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,9 +57,17 @@ export class CardObserver {
return;
}
this.intersectionCallback = intersectionCallback;

const init: IntersectionObserverInit = {
threshold: 0,
root: this.root ?? null,
};
if (this.buffer) {
init.rootMargin = this.buffer;
}
this.intersectionObserver = new IntersectionObserver(
this.onCardIntersection.bind(this),
{threshold: 0, root: this.root, rootMargin: this.buffer}
init
);
}

Expand Down
Loading

0 comments on commit 6fb4cad

Please sign in to comment.