Stop removing experiment related query parameters

tensorboard/webapp/app_routing/location.ts

@@ -48,9 +48,6 @@ const utils = {
   getHref() {
     return window.location.href;
   },
-  getSearch() {
-    return window.location.search;
-  },
 };
 
 @Injectable()
@@ -60,7 +57,7 @@ export class Location implements LocationInterface {
   }
 
   getSearch(): SerializableQueryParams {
-    const searchParams = new URLSearchParams(utils.getSearch());
+    const searchParams = new URLSearchParams(window.location.search);
     const serializableSearchParams: SerializableQueryParams = [];
 
     // URLSearchParams is a Iterable but TypeScript does not know about that.

tensorboard/webapp/feature_flag/store/BUILD

@@ -10,6 +10,7 @@ tf_ng_module(
         "feature_flag_store_config_provider.ts",
     ],
     deps = [
+        ":feature_flag_metadata",
         ":types",
         "//tensorboard/webapp/feature_flag:types",
         "//tensorboard/webapp/feature_flag/actions",

tensorboard/webapp/feature_flag/store/feature_flag_metadata.ts

@@ -30,8 +30,7 @@ export type BaseFeatureFlagType = boolean | number | string | null | undefined;
 export type FeatureFlagType = BaseFeatureFlagType | Array<BaseFeatureFlagType>;
 
 export type FeatureFlagMetadata<T> = {
-  displayName: string;
-  defaultValue?: T;
+  defaultValue: T;
   queryParamOverride?: string;
   parseValue: (str: string) => T;
   isArray?: boolean;
@@ -52,71 +51,70 @@ export const FeatureFlagMetadataMap: {
   [FlagName in keyof FeatureFlags]: FeatureFlagMetadata<FeatureFlags[FlagName]>;
 } = {
   scalarsBatchSize: {
-    displayName: 'scalarsBatchSize',
+    defaultValue: undefined,
     queryParamOverride: SCALARS_BATCH_SIZE_PARAM_KEY,
     parseValue: parseInt,
   },
   enabledColorGroup: {
-    displayName: 'enabledColorGroup',
+    defaultValue: true,
     queryParamOverride: ENABLE_COLOR_GROUP_QUERY_PARAM_KEY,
     parseValue: parseBoolean,
   },
   enabledColorGroupByRegex: {
-    displayName: 'enabledColorGroupByRegex',
+    defaultValue: true,
     queryParamOverride: ENABLE_COLOR_GROUP_BY_REGEX_QUERY_PARAM_KEY,
     parseValue: parseBoolean,
   },
   enabledExperimentalPlugins: {
-    displayName: 'enabledExperimentalPlugins',
+    defaultValue: [],
     queryParamOverride: EXPERIMENTAL_PLUGIN_QUERY_PARAM_KEY,
     parseValue: (str: string) => [str],
     isArray: true,
   },
   enabledLinkedTime: {
-    displayName: 'enabledLinkedTime',
+    defaultValue: false,
     queryParamOverride: ENABLE_LINKED_TIME_PARAM_KEY,
     parseValue: parseBoolean,
   },
   enabledCardWidthSetting: {
-    displayName: 'enabledCardWidthSetting',
+    defaultValue: true,
     queryParamOverride: ENABLE_CARD_WIDTH_SETTING_PARAM_KEY,
     parseValue: parseBoolean,
   },
   enabledScalarDataTable: {
-    displayName: 'enabledScalarDataTable',
+    defaultValue: false,
     queryParamOverride: ENABLE_DATA_TABLE_PARAM_KEY,
     parseValue: parseBoolean,
   },
   forceSvg: {
-    displayName: 'forceSvg',
+    defaultValue: false,
     queryParamOverride: FORCE_SVG_RENDERER,
     parseValue: parseBoolean,
   },
   enableDarkModeOverride: {
-    displayName: 'enableDarkModeOverride',
+    defaultValue: null,
     parseValue: parseBooleanOrNull,
   },
   defaultEnableDarkMode: {
-    displayName: 'defaultEnableDarkMode',
+    defaultValue: false,
     queryParamOverride: ENABLE_DARK_MODE_QUERY_PARAM_KEY,
     parseValue: parseBoolean,
   },
   isAutoDarkModeAllowed: {
-    displayName: 'isAutoDarkModeAllowed',
+    defaultValue: true,
     parseValue: parseBoolean,
   },
   inColab: {
-    displayName: 'inColab',
     defaultValue: false,
     queryParamOverride: 'tensorboardColab',
     parseValue: parseBoolean,
   },
   metricsImageSupportEnabled: {
-    displayName: 'metricsImageSupportEnabled',
+    defaultValue: true,
     parseValue: parseBoolean,
   },
   enableTimeSeriesPromotion: {
-    displayName: 'enableTimeSeriesPromotion',
+    defaultValue: false,
     parseValue: parseBoolean,
   },
 };

tensorboard/webapp/feature_flag/store/feature_flag_store_config_provider.ts

@@ -14,26 +14,21 @@ limitations under the License.
 ==============================================================================*/
 import {InjectionToken} from '@angular/core';
 import {StoreConfig} from '@ngrx/store';
+import {FeatureFlags} from '../types';
+import {FeatureFlagMetadataMap} from './feature_flag_metadata';
 import {FeatureFlagState} from './feature_flag_types';
 
+const defaultFlags = Object.entries(FeatureFlagMetadataMap).reduce(
+  (map, [key, {defaultValue}]) => {
+    map[key] = defaultValue;
+    return map;
+  },
+  {} as Record<string, any>
+) as FeatureFlags;
+
 export const initialState: FeatureFlagState = {
   isFeatureFlagsLoaded: false,
-  defaultFlags: {
-    isAutoDarkModeAllowed: true,
-    defaultEnableDarkMode: false,
-    enableDarkModeOverride: null,
-    enabledColorGroup: true,
-    enabledColorGroupByRegex: true,
-    enabledExperimentalPlugins: [],
-    inColab: false,
-    scalarsBatchSize: undefined,
-    metricsImageSupportEnabled: true,
-    enabledLinkedTime: false,
-    enableTimeSeriesPromotion: false,
-    enabledCardWidthSetting: true,
-    forceSvg: false,
-    enabledScalarDataTable: false,
-  },
+  defaultFlags,
   flagOverrides: {},
 };
 

tensorboard/webapp/routes/dashboard_deeplink_provider.ts

@@ -24,10 +24,7 @@ import {
   FeatureFlagMetadataMap,
   FeatureFlagType,
 } from '../feature_flag/store/feature_flag_metadata';
-import {
-  getEnabledExperimentalPlugins,
-  getOverriddenFeatureFlags,
-} from '../feature_flag/store/feature_flag_selectors';
+import {getOverriddenFeatureFlags} from '../feature_flag/store/feature_flag_selectors';
 import {
   isPluginType,
   isSampledPlugin,
@@ -101,15 +98,6 @@ export class DashboardDeepLinkProvider extends DeepLinkProvider {
           return [{key: TAG_FILTER_KEY, value: filterText}];
         })
       ),
-      store.select(getEnabledExperimentalPlugins).pipe(
-        map((enabledExperimentalPlugins) => {
-          return enabledExperimentalPlugins.map((pluginName) => ({
-            key: FeatureFlagMetadataMap.enabledExperimentalPlugins
-              .queryParamOverride!,
-            value: pluginName,
-          }));
-        })
-      ),
       store.select(getOverriddenFeatureFlags).pipe(
         map((featureFlags) => {
           return featureFlagsToSerializableQueryParams(

tensorboard/webapp/routes/dashboard_deeplink_provider_test.ts

@@ -347,13 +347,15 @@ describe('core deeplink provider', () => {
     });
   });
 
+  /**
+   * These tests are intended to verify that feature flags are correctly serialized using
+   * featureFlagsToSerializableQueryParams
+   */
   describe('feature flag', () => {
     it('serializes enabled experimental plugins', () => {
-      store.overrideSelector(selectors.getEnabledExperimentalPlugins, [
-        'foo',
-        'bar',
-        'baz',
-      ]);
+      store.overrideSelector(selectors.getOverriddenFeatureFlags, {
+        enabledExperimentalPlugins: ['foo', 'bar', 'baz'],
+      });
       store.refreshState();
 
       expect(queryParamsSerialized[queryParamsSerialized.length - 1]).toEqual([
@@ -369,18 +371,18 @@ describe('core deeplink provider', () => {
       });
       store.refreshState();
 
-      expect(queryParamsSerialized[queryParamsSerialized.length - 1]).toEqual([
-        {key: 'enableColorGroup', value: 'true'},
-      ]);
+      expect(queryParamsSerialized[queryParamsSerialized.length - 1]).toEqual(
+        []
+      );
 
       store.overrideSelector(selectors.getOverriddenFeatureFlags, {
         enabledColorGroup: false,
       });
       store.refreshState();
 
-      expect(queryParamsSerialized[queryParamsSerialized.length - 1]).toEqual(
-        []
-      );
+      expect(queryParamsSerialized[queryParamsSerialized.length - 1]).toEqual([
+        {key: 'enableColorGroup', value: 'false'},
+      ]);
 
       store.overrideSelector(selectors.getOverriddenFeatureFlags, {});
       store.refreshState();
@@ -396,18 +398,18 @@ describe('core deeplink provider', () => {
       });
       store.refreshState();
 
-      expect(queryParamsSerialized[queryParamsSerialized.length - 1]).toEqual([
-        {key: 'enableColorGroupByRegex', value: 'true'},
-      ]);
+      expect(queryParamsSerialized[queryParamsSerialized.length - 1]).toEqual(
+        []
+      );
 
       store.overrideSelector(selectors.getOverriddenFeatureFlags, {
         enabledColorGroupByRegex: false,
       });
       store.refreshState();
 
-      expect(queryParamsSerialized[queryParamsSerialized.length - 1]).toEqual(
-        []
-      );
+      expect(queryParamsSerialized[queryParamsSerialized.length - 1]).toEqual([
+        {key: 'enableColorGroupByRegex', value: 'false'},
+      ]);
 
       store.overrideSelector(selectors.getOverriddenFeatureFlags, {});
       store.refreshState();

tensorboard/webapp/routes/feature_flag_serializer.ts

@@ -1,8 +1,27 @@
+/* Copyright 2022 The TensorFlow Authors. All Rights Reserved.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+==============================================================================*/
 import {SerializableQueryParams} from '../app_routing/types';
-import {FeatureFlagMetadata} from '../feature_flag/store/feature_flag_metadata';
+import {
+  FeatureFlagMetadata,
+  FeatureFlagType,
+} from '../feature_flag/store/feature_flag_metadata';
 import {FeatureFlags} from '../feature_flag/types';
 
-export function featureFlagsToSerializableQueryParams<T>(
+export function featureFlagsToSerializableQueryParams<
+  T extends FeatureFlagType
+>(
   overriddenFeatureFlags: Partial<FeatureFlags>,
   featureFlagMetadataMap: Record<string, FeatureFlagMetadata<T>>
 ): SerializableQueryParams {
@@ -11,7 +30,9 @@ export function featureFlagsToSerializableQueryParams<T>(
       const key =
         featureFlagMetadataMap[featureFlag as keyof FeatureFlags]
           ?.queryParamOverride;
-      if (!key || !featureValue) {
+      const defaultValue =
+        featureFlagMetadataMap[featureFlag as keyof FeatureFlags]?.defaultValue;
+      if (!key || featureValue === undefined || featureValue === defaultValue) {
         return [];
       }
       /**

tensorboard/webapp/routes/feature_flag_serializer_test.ts

@@ -1,3 +1,17 @@
+/* Copyright 2022 The TensorFlow Authors. All Rights Reserved.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+==============================================================================*/
 import {featureFlagsToSerializableQueryParams} from './feature_flag_serializer';
 
 describe('feature flag serializer', () => {
@@ -9,15 +23,18 @@ describe('feature flag serializer', () => {
           displayName: 'featureA',
           queryParamOverride: 'feature_a',
           parseValue: (s: string) => s,
+          defaultValue: 'feature_a_123',
         },
         featureB: {
           displayName: 'featureB',
           queryParamOverride: 'feature_b',
           isArray: true,
           parseValue: (s: string) => s,
+          defaultValue: 'feature_b_456',
         },
       };
     });
+
     it('should return empty list when no flags are overridden', () => {
       const serializableQueryParams = featureFlagsToSerializableQueryParams(
         {},
@@ -44,6 +61,23 @@ describe('feature flag serializer', () => {
       ]);
     });
 
+    it('should serialize feature flags with falsy values', () => {
+      const serializableQueryParams = featureFlagsToSerializableQueryParams(
+        {featureB: false, featureA: ''} as any,
+        featureFlagsMetadata
+      );
+      expect(serializableQueryParams).toEqual([
+        {
+          key: 'feature_b',
+          value: 'false',
+        },
+        {
+          key: 'feature_a',
+          value: '',
+        },
+      ]);
+    });
+
     it('should return multiple entries for features with array values', () => {
       const serializableQueryParams = featureFlagsToSerializableQueryParams(
         {featureA: 'a', featureB: ['foo', 'bar']} as any,
@@ -64,5 +98,18 @@ describe('feature flag serializer', () => {
         },
       ]);
     });
+
+    it('should not serialize feaure flags with default values', () => {
+      const serializableQueryParams = featureFlagsToSerializableQueryParams(
+        {featureA: 'feature_a_123', featureB: 'foo'} as any,
+        featureFlagsMetadata
+      );
+      expect(serializableQueryParams).toEqual([
+        {
+          key: 'feature_b',
+          value: 'foo',
+        },
+      ]);
+    });
   });
 });

tensorboard/webapp/webapp_data_source/tb_feature_flag_data_source.ts

@@ -60,9 +60,6 @@ export class QueryParamsFeatureFlagDataSource
       return null;
     }
     const paramValues: T[] = params.getAll(queryParamOverride).map((value) => {
-      if (value === '' && flagMetadata.defaultValue !== undefined) {
-        return flagMetadata.defaultValue;
-      }
       return flagMetadata.parseValue(value);
     });
     if (!paramValues.length) {