diff --git a/tensorboard/plugins/debugger_v2/tf_debugger_v2_plugin/views/graph_executions/graph_executions_container.ts b/tensorboard/plugins/debugger_v2/tf_debugger_v2_plugin/views/graph_executions/graph_executions_container.ts index 611e0200db..b2d1e0c349 100644 --- a/tensorboard/plugins/debugger_v2/tf_debugger_v2_plugin/views/graph_executions/graph_executions_container.ts +++ b/tensorboard/plugins/debugger_v2/tf_debugger_v2_plugin/views/graph_executions/graph_executions_container.ts @@ -42,33 +42,19 @@ import {State} from '../../store/debugger_types'; `, }) export class GraphExecutionsContainer { - readonly numGraphExecutions$ = this.store.pipe(select(getNumGraphExecutions)); + readonly numGraphExecutions$; - readonly graphExecutionData$ = this.store.pipe(select(getGraphExecutionData)); + readonly graphExecutionData$; - readonly graphExecutionIndices$ = this.store.pipe( - select( - createSelector( - getNumGraphExecutions, - (numGraphExecution: number): number[] | null => { - if (numGraphExecution === 0) { - return null; - } - return Array.from({length: numGraphExecution}).map((_, i) => i); - } - ) - ) - ); + readonly graphExecutionIndices$; - readonly focusIndex$ = this.store.pipe(select(getGraphExecutionFocusIndex)); + readonly focusIndex$; /** * Inferred graph-execution indices that belong to the immediate inputs * to the currently-focused graph op. */ - readonly focusInputIndices$ = this.store.pipe( - select(getFocusedGraphExecutionInputIndices) - ); + readonly focusInputIndices$; onScrolledIndexChange(scrolledIndex: number) { this.store.dispatch(graphExecutionScrollToIndex({index: scrolledIndex})); @@ -78,5 +64,25 @@ export class GraphExecutionsContainer { this.store.dispatch(graphExecutionFocused(event)); } - constructor(private readonly store: Store) {} + constructor(private readonly store: Store) { + this.numGraphExecutions$ = this.store.pipe(select(getNumGraphExecutions)); + this.graphExecutionData$ = this.store.pipe(select(getGraphExecutionData)); + this.graphExecutionIndices$ = this.store.pipe( + select( + createSelector( + getNumGraphExecutions, + (numGraphExecution: number): number[] | null => { + if (numGraphExecution === 0) { + return null; + } + return Array.from({length: numGraphExecution}).map((_, i) => i); + } + ) + ) + ); + this.focusIndex$ = this.store.pipe(select(getGraphExecutionFocusIndex)); + this.focusInputIndices$ = this.store.pipe( + select(getFocusedGraphExecutionInputIndices) + ); + } } diff --git a/tensorboard/webapp/app_routing/effects/app_routing_effects.ts b/tensorboard/webapp/app_routing/effects/app_routing_effects.ts index 7ade04f5a6..e280268577 100644 --- a/tensorboard/webapp/app_routing/effects/app_routing_effects.ts +++ b/tensorboard/webapp/app_routing/effects/app_routing_effects.ts @@ -135,14 +135,7 @@ export class AppRoutingEffects { private readonly programmaticalNavModule: ProgrammaticalNavigationModule, private readonly appRootProvider: AppRootProvider ) { - this.routeConfigs = registry.getRouteConfigs(); - } - - /** - * Generates InternalNavigation from navigationRequested action. - */ - private readonly onNavigationRequested$: Observable = - this.actions$.pipe( + this.onNavigationRequested$ = this.actions$.pipe( ofType(navigationRequested), map((navigation) => { const resolvedPathname = navigation.pathname.startsWith('/') @@ -162,27 +155,17 @@ export class AppRoutingEffects { }; }) ); - - /** - * @exports - */ - readonly bootstrapReducers$ = createEffect(() => { - return this.actions$.pipe( - ofType(initAction), - map(() => { - return routeConfigLoaded({ - routeKinds: new Set(this.registry.getRegisteredRouteKinds()), - }); - }) - ); - }); - - /** - * Generates InternalNavigation from application load. - */ - private readonly onInit$: Observable = this.actions$ - .pipe(ofType(initAction)) - .pipe( + this.bootstrapReducers$ = createEffect(() => { + return this.actions$.pipe( + ofType(initAction), + map(() => { + return routeConfigLoaded({ + routeKinds: new Set(this.registry.getRegisteredRouteKinds()), + }); + }) + ); + }); + this.onInit$ = this.actions$.pipe(ofType(initAction)).pipe( delay(0), map(() => { const namespaceId: string | undefined = @@ -214,13 +197,7 @@ export class AppRoutingEffects { }; }) ); - - /** - * Generates InternalNavigation from browser history popstate event. - */ - private readonly onPopState$: Observable = this.location - .onPopState() - .pipe( + this.onPopState$ = this.location.onPopState().pipe( map((navigation) => { const namespaceUpdate: NamespaceUpdate = navigation.state?.namespaceId === undefined @@ -254,6 +231,344 @@ export class AppRoutingEffects { }; }) ); + this.userInitNavRoute$ = merge( + this.onNavigationRequested$, + this.onInit$, + this.onPopState$ + ).pipe( + map((navigation) => { + // Expect to have absolute navigation here. + if (!navigation.pathname.startsWith('/')) { + throw new Error( + `[App routing] pathname must start with '/'. Got: ${navigation.pathname}` + ); + } + return { + ...navigation, + pathname: this.appRootProvider.getAppRootlessPathname( + navigation.pathname + ), + }; + }), + map((navigationWithAbsolutePath) => { + const routeMatch = this.routeConfigs.match(navigationWithAbsolutePath); + return { + routeMatch, + options: navigationWithAbsolutePath.options, + }; + }) + ); + this.programmaticalNavRoute$ = this.actions$.pipe( + map((action) => { + return this.programmaticalNavModule.getNavigation(action); + }), + filter((nav) => { + return nav !== null; + }), + map((programmaticalNavigation) => { + const nav = programmaticalNavigation!; + const {replaceState = false, resetNamespacedState, routeKind} = nav; + + // TODO(stephanwlee): currently, the RouteParams is ill-typed and you + // can currently add any property without any type error. Better type + // it. + let routeParams: RouteParams; + switch (nav.routeKind) { + case RouteKind.COMPARE_EXPERIMENT: + routeParams = { + experimentIds: serializeCompareExperimentParams( + nav.routeParams.aliasAndExperimentIds + ), + }; + break; + default: + routeParams = nav.routeParams; + } + return {replaceState, routeKind, routeParams, resetNamespacedState}; + }), + map(({replaceState, routeKind, routeParams, resetNamespacedState}) => { + const routeMatch = this.routeConfigs + ? this.routeConfigs.matchByRouteKind(routeKind, routeParams) + : null; + return { + routeMatch, + options: { + replaceState, + browserInitiated: false, + namespaceUpdate: { + option: resetNamespacedState + ? NamespaceUpdateOption.NEW + : NamespaceUpdateOption.UNCHANGED, + }, + } as NavigationOptions, + }; + }) + ); + this.validatedRouteMatch$ = merge( + this.userInitNavRoute$, + this.programmaticalNavRoute$ + ).pipe( + filter(({routeMatch}) => Boolean(routeMatch)), + map(({routeMatch, options}) => { + return { + routeMatch: routeMatch!, + options, + }; + }) + ); + this.navigate$ = createEffect(() => { + const dispatchNavigating$ = this.validatedRouteMatch$.pipe( + withLatestFrom(this.store.select(getActiveRoute)), + mergeMap(([internalRouteMatch, oldRoute]) => { + // Check for unsaved updates and only proceed if the user confirms they + // want to continue without saving. + const sameRouteAndExperiments = + oldRoute !== null && + areSameRouteKindAndExperiments( + oldRoute, + internalRouteMatch.routeMatch + ); + const dirtySelectors = + this.dirtyUpdatesRegistry.getDirtyUpdatesSelectors(); + // Do not warn about unsaved updates when route and experiments are the + // same (e.g. when changing tabs in the same experiment page or query + // params in experiment list). + if (sameRouteAndExperiments || !dirtySelectors.length) + return of(internalRouteMatch); + return forkJoin( + this.dirtyUpdatesRegistry + .getDirtyUpdatesSelectors() + .map((selector) => this.store.select(selector).pipe(take(1))) + ).pipe( + map( + (updates) => + updates[0].experimentIds !== undefined && + updates[0].experimentIds.length > 0 + ), + filter((hasDirtyUpdates) => { + if (hasDirtyUpdates) { + const discardChanges = window.confirm( + `You have unsaved edits, are you sure you want to discard them?` + ); + if (discardChanges) { + this.store.dispatch(discardDirtyUpdates()); + } + return discardChanges; + } + return true; + }), + map(() => { + return internalRouteMatch; + }) + ); + }), + withLatestFrom(this.store.select(getRehydratedDeepLinks)), + tap(([{routeMatch, options}, rehydratedDeepLinks]) => { + // Possibly rehydrate state from the URL. + + if (!options.browserInitiated || !routeMatch.deepLinkProvider) { + return; + } + + if ( + options.namespaceUpdate.option === + NamespaceUpdateOption.FROM_HISTORY && + !canRehydrateDeepLink( + routeMatch.routeKind, + options.namespaceUpdate.namespaceId, + rehydratedDeepLinks + ) + ) { + // A deeplink has already been rehydrated for this RouteKind/Namespace + // combination so don't do it again. + return; + } + + // Query parameter formed by the redirector is passed to the + // deserializer instead of one from Location.getSearch(). This + // behavior emulates redirected URL to be on the URL bar such as + // "/compare?foo=bar" based on information provided by redirector (do + // note that location.getSearch() will return current query parameter + // which is pre-redirection URL). + const queryParams = + routeMatch.originateFromRedirection && + routeMatch.redirectionOnlyQueryParams + ? routeMatch.redirectionOnlyQueryParams + : this.location.getSearch(); + const rehydratingState = + routeMatch.deepLinkProvider.deserializeQueryParams(queryParams); + this.store.dispatch( + stateRehydratedFromUrl({ + routeKind: routeMatch.routeKind, + partialState: rehydratingState, + }) + ); + }), + tap(([{routeMatch}]) => { + // Some route configurations can generate actions that should be + // dispatched early in app routing handling. + if (routeMatch.action) { + this.store.dispatch(routeMatch.action); + } + }), + switchMap(([{routeMatch, options}]): Observable => { + if (routeMatch.deepLinkProvider === null) { + // Without a DeepLinkProvider emit a single result without query + // params. + return of({ + route: { + routeKind: routeMatch.routeKind, + params: routeMatch.params, + }, + pathname: routeMatch.pathname, + queryParams: [], + options, + }); + } + + // With a DeepLinkProvider emit a new result each time the query + // params change. + return routeMatch + .deepLinkProvider!.serializeStateToQueryParams(this.store) + .pipe( + map((queryParams, index) => { + return { + route: { + routeKind: routeMatch.routeKind, + params: routeMatch.params, + }, + pathname: routeMatch.pathname, + queryParams, + // Only honor replaceState value on first emit. On subsequent + // emits we always want to replaceState rather than pushState. + options: + index === 0 + ? options + : { + ...options, + namespaceUpdate: { + option: NamespaceUpdateOption.UNCHANGED, + }, + replaceState: true, + }, + }; + }) + ); + }), + tap(({route}) => { + // b/160185039: Allows the route store + router outlet to change + // before the route change. Because we debounceTime, technically, it does + // not fire two actions sequentially. + this.store.dispatch(navigating({after: route})); + }), + // Inject some async-ness so: + // 1. the router-outlet flush the change in a microtask. + // 2. we do not have composite action (synchronous dispatchment of + // actions). + debounceTime(0) + ); + + const changeUrl$ = dispatchNavigating$.pipe( + withLatestFrom(this.store.select(getActiveRoute)), + map(([newRoute, oldRoute]) => { + // The URL hash can be set via HashStorageComponent (which uses + // Polymer's tf-storage). DeepLinkProviders also modify the URL when + // a provider's serializeStateToQueryParams() emits. These result in + // the URL updated without the previous hash. HashStorageComponent + // makes no attempt to restore the hash, so it is dropped. + + // This results in bad behavior when refreshing (e.g. lost active + // plugin) and when changing dashboards (e.g. lost tagFilter). + + // TODO(b/169799696): either AppRouting should manage the URL entirely + // (including hash), or we make the app wait for AppRouting to + // initialize before setting the active plugin hash. + // See https://github.com/tensorflow/tensorboard/issues/4207. + const preserveHash = + oldRoute === null || + newRoute.route === null || + areSameRouteKindAndExperiments(oldRoute, newRoute.route); + return { + ...newRoute, + preserveHash, + }; + }), + tap(({preserveHash, pathname, queryParams, options}) => { + const shouldUpdateHistory = !arePathsAndQueryParamsEqual( + {pathname, queryParams}, + { + pathname: this.appRootProvider.getAppRootlessPathname( + this.location.getPath() + ), + queryParams: this.location.getSearch(), + } + ); + if (!shouldUpdateHistory) return; + + if (options.replaceState) { + this.location.replaceStateUrl( + this.appRootProvider.getAbsPathnameWithAppRoot( + this.location.getFullPath(pathname, queryParams, preserveHash) + ) + ); + } else { + this.location.pushStateUrl( + this.appRootProvider.getAbsPathnameWithAppRoot( + this.location.getFullPath(pathname, queryParams, preserveHash) + ) + ); + } + }) + ); + + return changeUrl$.pipe( + withLatestFrom( + this.store.select(getActiveRoute), + this.store.select(getActiveNamespaceId) + ), + map(([{route, options}, oldRoute, beforeNamespaceId]) => { + const afterNamespaceId = getAfterNamespaceId( + route, + options, + beforeNamespaceId + ); + + this.location.replaceStateData({ + ...this.location.getHistoryState(), + namespaceId: afterNamespaceId, + }); + + return navigated({ + before: oldRoute, + after: route, + beforeNamespaceId, + afterNamespaceId, + }); + }) + ); + }); + this.routeConfigs = registry.getRouteConfigs(); + } + + /** + * Generates InternalNavigation from navigationRequested action. + */ + private readonly onNavigationRequested$: Observable; + + /** + * @export + */ + readonly bootstrapReducers$; + + /** + * Generates InternalNavigation from application load. + */ + private readonly onInit$: Observable; + + /** + * Generates InternalNavigation from browser history popstate event. + */ + private readonly onPopState$: Observable; /** * Generates an InternalRouteMatch from the following events: @@ -265,33 +580,7 @@ export class AppRoutingEffects { * The input InternalNavigation values must have absolute pathname with * appRoot prefixed (e.g., window.location.pathname) when appRoot is defined. */ - private readonly userInitNavRoute$ = merge( - this.onNavigationRequested$, - this.onInit$, - this.onPopState$ - ).pipe( - map((navigation) => { - // Expect to have absolute navigation here. - if (!navigation.pathname.startsWith('/')) { - throw new Error( - `[App routing] pathname must start with '/'. Got: ${navigation.pathname}` - ); - } - return { - ...navigation, - pathname: this.appRootProvider.getAppRootlessPathname( - navigation.pathname - ), - }; - }), - map((navigationWithAbsolutePath) => { - const routeMatch = this.routeConfigs.match(navigationWithAbsolutePath); - return { - routeMatch, - options: navigationWithAbsolutePath.options, - }; - }) - ); + private readonly userInitNavRoute$; /** * Generates an InternalNavigation then InternalRouteMatch for programmatical @@ -299,304 +588,18 @@ export class AppRoutingEffects { * * See: ProgrammaticalNavigationModule. */ - private readonly programmaticalNavRoute$ = this.actions$.pipe( - map((action) => { - return this.programmaticalNavModule.getNavigation(action); - }), - filter((nav) => { - return nav !== null; - }), - map((programmaticalNavigation) => { - const nav = programmaticalNavigation!; - const {replaceState = false, resetNamespacedState, routeKind} = nav; - - // TODO(stephanwlee): currently, the RouteParams is ill-typed and you - // can currently add any property without any type error. Better type - // it. - let routeParams: RouteParams; - switch (nav.routeKind) { - case RouteKind.COMPARE_EXPERIMENT: - routeParams = { - experimentIds: serializeCompareExperimentParams( - nav.routeParams.aliasAndExperimentIds - ), - }; - break; - default: - routeParams = nav.routeParams; - } - return {replaceState, routeKind, routeParams, resetNamespacedState}; - }), - map(({replaceState, routeKind, routeParams, resetNamespacedState}) => { - const routeMatch = this.routeConfigs - ? this.routeConfigs.matchByRouteKind(routeKind, routeParams) - : null; - return { - routeMatch, - options: { - replaceState, - browserInitiated: false, - namespaceUpdate: { - option: resetNamespacedState - ? NamespaceUpdateOption.NEW - : NamespaceUpdateOption.UNCHANGED, - }, - } as NavigationOptions, - }; - }) - ); + private readonly programmaticalNavRoute$; /** * Merges all the event paths, ensuring they have generated a valid * InternalRouteMatch. */ - private readonly validatedRouteMatch$: Observable = merge( - this.userInitNavRoute$, - this.programmaticalNavRoute$ - ).pipe( - filter(({routeMatch}) => Boolean(routeMatch)), - map(({routeMatch, options}) => { - return { - routeMatch: routeMatch!, - options, - }; - }) - ); + private readonly validatedRouteMatch$: Observable; /** * @export */ - navigate$ = createEffect(() => { - const dispatchNavigating$ = this.validatedRouteMatch$.pipe( - withLatestFrom(this.store.select(getActiveRoute)), - mergeMap(([internalRouteMatch, oldRoute]) => { - // Check for unsaved updates and only proceed if the user confirms they - // want to continue without saving. - const sameRouteAndExperiments = - oldRoute !== null && - areSameRouteKindAndExperiments( - oldRoute, - internalRouteMatch.routeMatch - ); - const dirtySelectors = - this.dirtyUpdatesRegistry.getDirtyUpdatesSelectors(); - // Do not warn about unsaved updates when route and experiments are the - // same (e.g. when changing tabs in the same experiment page or query - // params in experiment list). - if (sameRouteAndExperiments || !dirtySelectors.length) - return of(internalRouteMatch); - return forkJoin( - this.dirtyUpdatesRegistry - .getDirtyUpdatesSelectors() - .map((selector) => this.store.select(selector).pipe(take(1))) - ).pipe( - map( - (updates) => - updates[0].experimentIds !== undefined && - updates[0].experimentIds.length > 0 - ), - filter((hasDirtyUpdates) => { - if (hasDirtyUpdates) { - const discardChanges = window.confirm( - `You have unsaved edits, are you sure you want to discard them?` - ); - if (discardChanges) { - this.store.dispatch(discardDirtyUpdates()); - } - return discardChanges; - } - return true; - }), - map(() => { - return internalRouteMatch; - }) - ); - }), - withLatestFrom(this.store.select(getRehydratedDeepLinks)), - tap(([{routeMatch, options}, rehydratedDeepLinks]) => { - // Possibly rehydrate state from the URL. - - if (!options.browserInitiated || !routeMatch.deepLinkProvider) { - return; - } - - if ( - options.namespaceUpdate.option === - NamespaceUpdateOption.FROM_HISTORY && - !canRehydrateDeepLink( - routeMatch.routeKind, - options.namespaceUpdate.namespaceId, - rehydratedDeepLinks - ) - ) { - // A deeplink has already been rehydrated for this RouteKind/Namespace - // combination so don't do it again. - return; - } - - // Query parameter formed by the redirector is passed to the - // deserializer instead of one from Location.getSearch(). This - // behavior emulates redirected URL to be on the URL bar such as - // "/compare?foo=bar" based on information provided by redirector (do - // note that location.getSearch() will return current query parameter - // which is pre-redirection URL). - const queryParams = - routeMatch.originateFromRedirection && - routeMatch.redirectionOnlyQueryParams - ? routeMatch.redirectionOnlyQueryParams - : this.location.getSearch(); - const rehydratingState = - routeMatch.deepLinkProvider.deserializeQueryParams(queryParams); - this.store.dispatch( - stateRehydratedFromUrl({ - routeKind: routeMatch.routeKind, - partialState: rehydratingState, - }) - ); - }), - tap(([{routeMatch}]) => { - // Some route configurations can generate actions that should be - // dispatched early in app routing handling. - if (routeMatch.action) { - this.store.dispatch(routeMatch.action); - } - }), - switchMap(([{routeMatch, options}]): Observable => { - if (routeMatch.deepLinkProvider === null) { - // Without a DeepLinkProvider emit a single result without query - // params. - return of({ - route: { - routeKind: routeMatch.routeKind, - params: routeMatch.params, - }, - pathname: routeMatch.pathname, - queryParams: [], - options, - }); - } - - // With a DeepLinkProvider emit a new result each time the query - // params change. - return routeMatch - .deepLinkProvider!.serializeStateToQueryParams(this.store) - .pipe( - map((queryParams, index) => { - return { - route: { - routeKind: routeMatch.routeKind, - params: routeMatch.params, - }, - pathname: routeMatch.pathname, - queryParams, - // Only honor replaceState value on first emit. On subsequent - // emits we always want to replaceState rather than pushState. - options: - index === 0 - ? options - : { - ...options, - namespaceUpdate: { - option: NamespaceUpdateOption.UNCHANGED, - }, - replaceState: true, - }, - }; - }) - ); - }), - tap(({route}) => { - // b/160185039: Allows the route store + router outlet to change - // before the route change. Because we debounceTime, technically, it does - // not fire two actions sequentially. - this.store.dispatch(navigating({after: route})); - }), - // Inject some async-ness so: - // 1. the router-outlet flush the change in a microtask. - // 2. we do not have composite action (synchronous dispatchment of - // actions). - debounceTime(0) - ); - - const changeUrl$ = dispatchNavigating$.pipe( - withLatestFrom(this.store.select(getActiveRoute)), - map(([newRoute, oldRoute]) => { - // The URL hash can be set via HashStorageComponent (which uses - // Polymer's tf-storage). DeepLinkProviders also modify the URL when - // a provider's serializeStateToQueryParams() emits. These result in - // the URL updated without the previous hash. HashStorageComponent - // makes no attempt to restore the hash, so it is dropped. - - // This results in bad behavior when refreshing (e.g. lost active - // plugin) and when changing dashboards (e.g. lost tagFilter). - - // TODO(b/169799696): either AppRouting should manage the URL entirely - // (including hash), or we make the app wait for AppRouting to - // initialize before setting the active plugin hash. - // See https://github.com/tensorflow/tensorboard/issues/4207. - const preserveHash = - oldRoute === null || - newRoute.route === null || - areSameRouteKindAndExperiments(oldRoute, newRoute.route); - return { - ...newRoute, - preserveHash, - }; - }), - tap(({preserveHash, pathname, queryParams, options}) => { - const shouldUpdateHistory = !arePathsAndQueryParamsEqual( - {pathname, queryParams}, - { - pathname: this.appRootProvider.getAppRootlessPathname( - this.location.getPath() - ), - queryParams: this.location.getSearch(), - } - ); - if (!shouldUpdateHistory) return; - - if (options.replaceState) { - this.location.replaceStateUrl( - this.appRootProvider.getAbsPathnameWithAppRoot( - this.location.getFullPath(pathname, queryParams, preserveHash) - ) - ); - } else { - this.location.pushStateUrl( - this.appRootProvider.getAbsPathnameWithAppRoot( - this.location.getFullPath(pathname, queryParams, preserveHash) - ) - ); - } - }) - ); - - return changeUrl$.pipe( - withLatestFrom( - this.store.select(getActiveRoute), - this.store.select(getActiveNamespaceId) - ), - map(([{route, options}, oldRoute, beforeNamespaceId]) => { - const afterNamespaceId = getAfterNamespaceId( - route, - options, - beforeNamespaceId - ); - - this.location.replaceStateData({ - ...this.location.getHistoryState(), - namespaceId: afterNamespaceId, - }); - - return navigated({ - before: oldRoute, - after: route, - beforeNamespaceId, - afterNamespaceId, - }); - }) - ); - }); + navigate$; /** @export */ ngrxOnInitEffects(): Action { diff --git a/tensorboard/webapp/app_routing/views/router_outlet_container.ts b/tensorboard/webapp/app_routing/views/router_outlet_container.ts index 8466fa9b48..0e56b6acaa 100644 --- a/tensorboard/webapp/app_routing/views/router_outlet_container.ts +++ b/tensorboard/webapp/app_routing/views/router_outlet_container.ts @@ -35,25 +35,27 @@ import { changeDetection: ChangeDetectionStrategy.OnPush, }) export class RouterOutletContainer { - activeNgComponent$ = combineLatest([ - this.store.select(getActiveRoute), - this.store.select(getNextRouteForRouterOutletOnly), - ]).pipe( - map(([activeRoute, nextRoute]) => { - if (!activeRoute) { - return null; - } - const isRouteTransitioning = - nextRoute !== null && - !areSameRouteKindAndExperiments(activeRoute, nextRoute); - return isRouteTransitioning - ? null - : this.registry.getNgComponentByRouteKind(activeRoute.routeKind); - }) - ); + activeNgComponent$; constructor( private readonly store: Store, private readonly registry: RouteRegistryModule - ) {} + ) { + this.activeNgComponent$ = combineLatest([ + this.store.select(getActiveRoute), + this.store.select(getNextRouteForRouterOutletOnly), + ]).pipe( + map(([activeRoute, nextRoute]) => { + if (!activeRoute) { + return null; + } + const isRouteTransitioning = + nextRoute !== null && + !areSameRouteKindAndExperiments(activeRoute, nextRoute); + return isRouteTransitioning + ? null + : this.registry.getNgComponentByRouteKind(activeRoute.routeKind); + }) + ); + } } diff --git a/tensorboard/webapp/core/views/layout_container.ts b/tensorboard/webapp/core/views/layout_container.ts index 1f2b8bbeac..0a37b546ee 100644 --- a/tensorboard/webapp/core/views/layout_container.ts +++ b/tensorboard/webapp/core/views/layout_container.ts @@ -80,21 +80,22 @@ import { changeDetection: ChangeDetectionStrategy.OnPush, }) export class LayoutContainer implements OnDestroy { - readonly runsTableFullScreen$ = this.store.select(getRunsTableFullScreen); - readonly width$: Observable = this.store - .select(getSideBarWidthInPercent) - .pipe( - combineLatestWith(this.runsTableFullScreen$), - map(([percentageWidth, fullScreen]) => { - return fullScreen ? 100 : percentageWidth; - }) - ); - private readonly ngUnsubscribe = new Subject(); + readonly runsTableFullScreen$; + readonly width$: Observable; + private readonly ngUnsubscribe; private resizing: boolean = false; readonly MINIMUM_SIDEBAR_WIDTH_IN_PX = 75; constructor(private readonly store: Store, hostElRef: ElementRef) { + this.runsTableFullScreen$ = this.store.select(getRunsTableFullScreen); + this.width$ = this.store.select(getSideBarWidthInPercent).pipe( + combineLatestWith(this.runsTableFullScreen$), + map(([percentageWidth, fullScreen]) => { + return fullScreen ? 100 : percentageWidth; + }) + ); + this.ngUnsubscribe = new Subject(); fromEvent(hostElRef.nativeElement, 'mousemove') .pipe( takeUntil(this.ngUnsubscribe), diff --git a/tensorboard/webapp/hparams/_redux/hparams_effects.ts b/tensorboard/webapp/hparams/_redux/hparams_effects.ts index 3c374ffc74..41bd358e0f 100644 --- a/tensorboard/webapp/hparams/_redux/hparams_effects.ts +++ b/tensorboard/webapp/hparams/_redux/hparams_effects.ts @@ -15,15 +15,15 @@ limitations under the License. import {Injectable} from '@angular/core'; import {Actions, createEffect, ofType} from '@ngrx/effects'; import {Store} from '@ngrx/store'; -import {Observable, of, throwError, merge} from 'rxjs'; +import {merge, Observable, of, throwError} from 'rxjs'; import { catchError, + distinctUntilChanged, filter, map, switchMap, - withLatestFrom, throttleTime, - distinctUntilChanged, + withLatestFrom, } from 'rxjs/operators'; import {navigated} from '../../app_routing/actions'; @@ -35,10 +35,10 @@ import {State} from '../../app_state'; import * as coreActions from '../../core/actions'; import {HttpErrorResponse} from '../../webapp_data_source/tb_http_client'; +import {RouteKind} from '../../app_routing/types'; +import {HparamSpec, SessionGroup} from '../types'; import * as hparamsActions from './hparams_actions'; import {HparamsDataSource} from './hparams_data_source'; -import {HparamSpec, SessionGroup} from '../types'; -import {RouteKind} from '../../app_routing/types'; import {getNumDashboardHparamsToLoad} from './hparams_selectors'; /** @@ -50,18 +50,15 @@ export class HparamsEffects { private readonly actions$: Actions, private readonly store: Store, private readonly dataSource: HparamsDataSource - ) {} - - private readonly navigated$: Observable = this.actions$.pipe( - ofType(navigated), - withLatestFrom(this.store.select(getExperimentIdsFromRoute)), - filter(([, experimentIds]) => Boolean(experimentIds)), - map(([, experimentIds]) => experimentIds as string[]), - distinctUntilChanged((prev, cur) => prev.join('') === cur.join('')) - ); - - private readonly loadHparamsOnReload$: Observable = - this.actions$.pipe( + ) { + this.navigated$ = this.actions$.pipe( + ofType(navigated), + withLatestFrom(this.store.select(getExperimentIdsFromRoute)), + filter(([, experimentIds]) => Boolean(experimentIds)), + map(([, experimentIds]) => experimentIds as string[]), + distinctUntilChanged((prev, cur) => prev.join('') === cur.join('')) + ); + this.loadHparamsOnReload$ = this.actions$.pipe( ofType( coreActions.reload, coreActions.manualReload, @@ -71,26 +68,32 @@ export class HparamsEffects { filter(([, experimentIds]) => Boolean(experimentIds)), map(([, experimentIds]) => experimentIds as string[]) ); + this.loadHparamsData$ = createEffect(() => { + return merge(this.navigated$, this.loadHparamsOnReload$).pipe( + withLatestFrom( + this.store.select(getActiveRoute), + this.store.select(getNumDashboardHparamsToLoad) + ), + filter( + ([, activeRoute]) => + activeRoute?.routeKind === RouteKind.EXPERIMENT || + activeRoute?.routeKind === RouteKind.COMPARE_EXPERIMENT + ), + throttleTime(10), + switchMap(([experimentIds, , numHparamsToLoad]) => + this.loadHparamsForExperiments(experimentIds, numHparamsToLoad) + ), + map((resp) => hparamsActions.hparamsFetchSessionGroupsSucceeded(resp)) + ); + }); + } + + private readonly navigated$: Observable; + + private readonly loadHparamsOnReload$: Observable; /** @export */ - loadHparamsData$ = createEffect(() => { - return merge(this.navigated$, this.loadHparamsOnReload$).pipe( - withLatestFrom( - this.store.select(getActiveRoute), - this.store.select(getNumDashboardHparamsToLoad) - ), - filter( - ([, activeRoute]) => - activeRoute?.routeKind === RouteKind.EXPERIMENT || - activeRoute?.routeKind === RouteKind.COMPARE_EXPERIMENT - ), - throttleTime(10), - switchMap(([experimentIds, , numHparamsToLoad]) => - this.loadHparamsForExperiments(experimentIds, numHparamsToLoad) - ), - map((resp) => hparamsActions.hparamsFetchSessionGroupsSucceeded(resp)) - ); - }); + loadHparamsData$; private loadHparamsForExperiments( experimentIds: string[], diff --git a/tensorboard/webapp/metrics/views/card_renderer/card_view_container.ts b/tensorboard/webapp/metrics/views/card_renderer/card_view_container.ts index bd9dff462a..22d1f77508 100644 --- a/tensorboard/webapp/metrics/views/card_renderer/card_view_container.ts +++ b/tensorboard/webapp/metrics/views/card_renderer/card_view_container.ts @@ -58,24 +58,8 @@ const RUN_COLOR_UPDATE_THROTTLE_TIME_IN_MS = 350; changeDetection: ChangeDetectionStrategy.OnPush, }) export class CardViewContainer { - constructor(private readonly store: Store) {} - - isEverVisible = false; - - @Input() cardId!: CardId; - @Input() groupName!: string | null; - @Input() pluginType!: PluginType; - - @Output() fullWidthChanged = new EventEmitter(); - @Output() fullHeightChanged = new EventEmitter(); - - onVisibilityChange({visible}: {visible: boolean}) { - this.isEverVisible = this.isEverVisible || visible; - } - - readonly runColorScale$: Observable = this.store - .select(selectors.getRunColorMap) - .pipe( + constructor(private readonly store: Store) { + this.runColorScale$ = this.store.select(selectors.getRunColorMap).pipe( throttleTime(RUN_COLOR_UPDATE_THROTTLE_TIME_IN_MS, undefined, { leading: true, trailing: true, @@ -91,6 +75,22 @@ export class CardViewContainer { }; }) ); + } + + isEverVisible = false; + + @Input() cardId!: CardId; + @Input() groupName!: string | null; + @Input() pluginType!: PluginType; + + @Output() fullWidthChanged = new EventEmitter(); + @Output() fullHeightChanged = new EventEmitter(); + + onVisibilityChange({visible}: {visible: boolean}) { + this.isEverVisible = this.isEverVisible || visible; + } + + readonly runColorScale$: Observable; onFullWidthChanged(showFullWidth: boolean) { this.fullWidthChanged.emit(showFullWidth); diff --git a/tensorboard/webapp/metrics/views/main_view/filtered_view_container.ts b/tensorboard/webapp/metrics/views/main_view/filtered_view_container.ts index 1c3131a3b3..5a6cf4c94e 100644 --- a/tensorboard/webapp/metrics/views/main_view/filtered_view_container.ts +++ b/tensorboard/webapp/metrics/views/main_view/filtered_view_container.ts @@ -51,49 +51,54 @@ export const FILTER_VIEW_DEBOUNCE_IN_MS = 200; export class FilteredViewContainer { @Input() cardObserver!: CardObserver; - constructor(private readonly store: Store) {} + constructor(private readonly store: Store) { + this.cardIdsWithMetadata$ = this.store + .select(getSortedRenderableCardIdsWithMetadata) + .pipe( + combineLatestWith(this.store.select(getMetricsFilteredPluginTypes)), + map(([cardList, filteredPluginTypes]) => { + if (!filteredPluginTypes.size) return cardList; + return cardList.filter((card) => + filteredPluginTypes.has(card.plugin) + ); + }), + combineLatestWith(this.store.select(getMetricsTagFilter)), + debounceTime(FILTER_VIEW_DEBOUNCE_IN_MS), + map(([cardList, tagFilter]) => { + try { + return {cardList, regex: new RegExp(tagFilter, 'i')}; + } catch (e) { + return {cardList, regex: null}; + } + }), + filter(({regex}) => regex !== null), + map(({cardList, regex}) => { + return cardList.filter(({tag}) => regex!.test(tag)); + }), + distinctUntilChanged[]>( + (prev, updated) => { + if (prev.length !== updated.length) { + return false; + } + return prev.every((prevVal, index) => { + return prevVal.cardId === updated[index].cardId; + }); + } + ), + share(), + startWith([]) + ) as Observable[]>; + this.isEmptyMatch$ = this.cardIdsWithMetadata$.pipe( + combineLatestWith( + this.store.select(getSortedRenderableCardIdsWithMetadata) + ), + map(([filteredCardList, fullCardList]) => { + return Boolean(fullCardList.length) && filteredCardList.length === 0; + }) + ); + } - readonly cardIdsWithMetadata$: Observable< - DeepReadonly[] - > = this.store.select(getSortedRenderableCardIdsWithMetadata).pipe( - combineLatestWith(this.store.select(getMetricsFilteredPluginTypes)), - map(([cardList, filteredPluginTypes]) => { - if (!filteredPluginTypes.size) return cardList; - return cardList.filter((card) => filteredPluginTypes.has(card.plugin)); - }), - combineLatestWith(this.store.select(getMetricsTagFilter)), - debounceTime(FILTER_VIEW_DEBOUNCE_IN_MS), - map(([cardList, tagFilter]) => { - try { - return {cardList, regex: new RegExp(tagFilter, 'i')}; - } catch (e) { - return {cardList, regex: null}; - } - }), - filter(({regex}) => regex !== null), - map(({cardList, regex}) => { - return cardList.filter(({tag}) => regex!.test(tag)); - }), - distinctUntilChanged[]>( - (prev, updated) => { - if (prev.length !== updated.length) { - return false; - } - return prev.every((prevVal, index) => { - return prevVal.cardId === updated[index].cardId; - }); - } - ), - share(), - startWith([]) - ) as Observable[]>; + readonly cardIdsWithMetadata$: Observable[]>; - readonly isEmptyMatch$: Observable = this.cardIdsWithMetadata$.pipe( - combineLatestWith( - this.store.select(getSortedRenderableCardIdsWithMetadata) - ), - map(([filteredCardList, fullCardList]) => { - return Boolean(fullCardList.length) && filteredCardList.length === 0; - }) - ); + readonly isEmptyMatch$: Observable; } diff --git a/tensorboard/webapp/metrics/views/main_view/pinned_view_container.ts b/tensorboard/webapp/metrics/views/main_view/pinned_view_container.ts index 6472e59ccb..0fa8836016 100644 --- a/tensorboard/webapp/metrics/views/main_view/pinned_view_container.ts +++ b/tensorboard/webapp/metrics/views/main_view/pinned_view_container.ts @@ -17,12 +17,12 @@ import {Store} from '@ngrx/store'; import {Observable} from 'rxjs'; import {skip, startWith} from 'rxjs/operators'; import {State} from '../../../app_state'; +import {getEnableGlobalPins} from '../../../selectors'; import {DeepReadonly} from '../../../util/types'; +import {metricsClearAllPinnedCards} from '../../actions'; import {getLastPinnedCardTime, getPinnedCardsWithMetadata} from '../../store'; import {CardObserver} from '../card_renderer/card_lazy_loader'; import {CardIdWithMetadata} from '../metrics_view_types'; -import {metricsClearAllPinnedCards} from '../../actions'; -import {getEnableGlobalPins} from '../../../selectors'; @Component({ standalone: false, @@ -41,19 +41,23 @@ import {getEnableGlobalPins} from '../../../selectors'; export class PinnedViewContainer { @Input() cardObserver!: CardObserver; - constructor(private readonly store: Store) {} + constructor(private readonly store: Store) { + this.cardIdsWithMetadata$ = this.store + .select(getPinnedCardsWithMetadata) + .pipe(startWith([])); + this.lastPinnedCardTime$ = this.store.select(getLastPinnedCardTime).pipe( + // Ignore the first value on component load, only reacting to new + // pins after page load. + skip(1) + ); + this.globalPinsEnabled$ = this.store.select(getEnableGlobalPins); + } - readonly cardIdsWithMetadata$: Observable< - DeepReadonly - > = this.store.select(getPinnedCardsWithMetadata).pipe(startWith([])); + readonly cardIdsWithMetadata$: Observable>; - readonly lastPinnedCardTime$ = this.store.select(getLastPinnedCardTime).pipe( - // Ignore the first value on component load, only reacting to new - // pins after page load. - skip(1) - ); + readonly lastPinnedCardTime$; - readonly globalPinsEnabled$ = this.store.select(getEnableGlobalPins); + readonly globalPinsEnabled$; onClearAllPinsClicked() { this.store.dispatch(metricsClearAllPinnedCards()); diff --git a/tensorboard/webapp/metrics/views/right_pane/scalar_column_editor/scalar_column_editor_container.ts b/tensorboard/webapp/metrics/views/right_pane/scalar_column_editor/scalar_column_editor_container.ts index 43bd157295..5421ce2df0 100644 --- a/tensorboard/webapp/metrics/views/right_pane/scalar_column_editor/scalar_column_editor_container.ts +++ b/tensorboard/webapp/metrics/views/right_pane/scalar_column_editor/scalar_column_editor_container.ts @@ -14,7 +14,12 @@ limitations under the License. ==============================================================================*/ import {ChangeDetectionStrategy, Component} from '@angular/core'; import {Store} from '@ngrx/store'; +import {map} from 'rxjs/operators'; import {State} from '../../../../app_state'; +import { + ColumnHeader, + DataTableMode, +} from '../../../../widgets/data_table/types'; import { dataTableColumnOrderChanged, dataTableColumnToggled, @@ -27,11 +32,6 @@ import { getTableEditorSelectedTab, } from '../../../store/metrics_selectors'; import {HeaderEditInfo, HeaderToggleInfo} from '../../../types'; -import { - ColumnHeader, - DataTableMode, -} from '../../../../widgets/data_table/types'; -import {map} from 'rxjs/operators'; function headersWithoutRuns(headers: ColumnHeader[]) { return headers.filter((header) => header.type !== 'RUN'); @@ -55,15 +55,19 @@ function headersWithoutRuns(headers: ColumnHeader[]) { changeDetection: ChangeDetectionStrategy.OnPush, }) export class ScalarColumnEditorContainer { - constructor(private readonly store: Store) {} + constructor(private readonly store: Store) { + this.singleHeaders$ = this.store + .select(getSingleSelectionHeaders) + .pipe(map(headersWithoutRuns)); + this.rangeHeaders$ = this.store + .select(getRangeSelectionHeaders) + .pipe(map(headersWithoutRuns)); + this.selectedTab$ = this.store.select(getTableEditorSelectedTab); + } - readonly singleHeaders$ = this.store - .select(getSingleSelectionHeaders) - .pipe(map(headersWithoutRuns)); - readonly rangeHeaders$ = this.store - .select(getRangeSelectionHeaders) - .pipe(map(headersWithoutRuns)); - readonly selectedTab$ = this.store.select(getTableEditorSelectedTab); + readonly singleHeaders$; + readonly rangeHeaders$; + readonly selectedTab$; onScalarTableColumnToggled(toggleInfo: HeaderToggleInfo) { this.store.dispatch(dataTableColumnToggled(toggleInfo)); diff --git a/tensorboard/webapp/notification_center/_views/notification_center_container.ts b/tensorboard/webapp/notification_center/_views/notification_center_container.ts index 186c43d096..2d8bbb25a1 100644 --- a/tensorboard/webapp/notification_center/_views/notification_center_container.ts +++ b/tensorboard/webapp/notification_center/_views/notification_center_container.ts @@ -39,8 +39,12 @@ const iconMap = new Map([[CategoryEnum.WHATS_NEW, 'info_outline_24px']]); `, }) export class NotificationCenterContainer { - readonly notificationNotes$: Observable = - combineLatest([ + readonly notificationNotes$: Observable; + + readonly hasUnreadMessages$; + + constructor(private readonly store: Store) { + this.notificationNotes$ = combineLatest([ this.store.select(getNotifications), this.store.select(getLastReadTime), ]).pipe( @@ -55,14 +59,12 @@ export class NotificationCenterContainer { }), shareReplay() ); - - readonly hasUnreadMessages$ = this.notificationNotes$.pipe( - map((notifications) => { - return notifications.some(({hasRead}) => !hasRead); - }) - ); - - constructor(private readonly store: Store) {} + this.hasUnreadMessages$ = this.notificationNotes$.pipe( + map((notifications) => { + return notifications.some(({hasRead}) => !hasRead); + }) + ); + } onBellButtonClicked() { this.store.dispatch(actions.notificationBellClicked()); diff --git a/tensorboard/webapp/runs/views/runs_table/runs_group_menu_button_container.ts b/tensorboard/webapp/runs/views/runs_table/runs_group_menu_button_container.ts index 2375cb0d71..a858b9338c 100644 --- a/tensorboard/webapp/runs/views/runs_table/runs_group_menu_button_container.ts +++ b/tensorboard/webapp/runs/views/runs_table/runs_group_menu_button_container.ts @@ -15,13 +15,13 @@ limitations under the License. import {ChangeDetectionStrategy, Component, Input} from '@angular/core'; import {Store} from '@ngrx/store'; import {Observable} from 'rxjs'; -import {map, take, filter, startWith, shareReplay} from 'rxjs/operators'; +import {filter, map, shareReplay, startWith, take} from 'rxjs/operators'; import {RouteKind} from '../../../app_routing/types'; import {State} from '../../../app_state'; import { - getRegisteredRouteKinds, getDashboardExperimentNames, getEnableColorByExperiment, + getRegisteredRouteKinds, } from '../../../selectors'; import {runGroupByChanged} from '../../actions'; import { @@ -51,35 +51,35 @@ import {GroupBy, GroupByKey} from '../../types'; export class RunsGroupMenuButtonContainer { @Input() experimentIds!: string[]; - constructor(private readonly store: Store) {} - - readonly showExperimentsGroupBy$: Observable = this.store - .select(getRegisteredRouteKinds) - .pipe( - map((registeredRouteKinds) => { - return registeredRouteKinds.has(RouteKind.COMPARE_EXPERIMENT); - }) - ); - - readonly selectedGroupBy$: Observable = - this.store.select(getRunGroupBy); - - readonly lastRegexGroupByKey$: Observable = this.store - .select(getRunGroupBy) - .pipe( + constructor(private readonly store: Store) { + this.showExperimentsGroupBy$ = this.store + .select(getRegisteredRouteKinds) + .pipe( + map((registeredRouteKinds) => { + return registeredRouteKinds.has(RouteKind.COMPARE_EXPERIMENT); + }) + ); + this.selectedGroupBy$ = this.store.select(getRunGroupBy); + this.lastRegexGroupByKey$ = this.store.select(getRunGroupBy).pipe( map((group) => group.key), filter( (key) => key === GroupByKey.REGEX || key === GroupByKey.REGEX_BY_EXP ), startWith(GroupByKey.REGEX) ); + this.groupByRegexString$ = this.store.select(getColorGroupRegexString); + this.expNameByExpId$ = this.store.select(getDashboardExperimentNames); + } + + readonly showExperimentsGroupBy$: Observable; + + readonly selectedGroupBy$: Observable; + + readonly lastRegexGroupByKey$: Observable; - readonly groupByRegexString$: Observable = this.store.select( - getColorGroupRegexString - ); + readonly groupByRegexString$: Observable; - readonly expNameByExpId$: Observable> = - this.store.select(getDashboardExperimentNames); + readonly expNameByExpId$: Observable>; onGroupByChange(groupBy: GroupBy) { this.expNameByExpId$.pipe(take(1)).subscribe((expNameByExpId) => {