♻️ Enable passing type-checking on src/experiments and src/examiner (a…

…mpproject#34394)

* mv src/experiments.js -> src/experiments/index.js

* Make src/experiments pass type-checking

* Allow use of window in extern files

* Remove unused import

* Allow AMP_CONFIG in extern

* Modernize experiments code

* Enable passing type-checking on src/examiner + modernize

* Use assert via logger

* un-hoist

* Clean up extra type annotations

* Fix tests passing object instead of array

ads/google/a4a/traffic-experiments.js

@@ -27,9 +27,6 @@ import {
   EXPERIMENT_ATTRIBUTE,
   mergeExperimentIds,
 } from './utils';
-import {
-  ExperimentInfo, // eslint-disable-line no-unused-vars
-} from '../../../src/experiments';
 import {Services} from '../../../src/services';
 import {parseQueryString} from '../../../src/url';
 

build-system/tasks/check-types.js

@@ -119,11 +119,10 @@ const TYPE_CHECK_TARGETS = {
   },
   'src-examiner': {
     srcGlobs: ['src/examiner/**/*.js'],
-    warningLevel: 'QUIET',
   },
   'src-experiments': {
-    srcGlobs: ['src/experiments/**/*.js'],
-    warningLevel: 'QUIET',
+    srcGlobs: ['src/experiments/**/*.js', ...CORE_SRCS_GLOBS],
+    externGlobs: ['src/experiments/**/*.extern.js', ...CORE_EXTERNS_GLOBS],
   },
   'src-inabox': {
     srcGlobs: ['src/inabox/**/*.js'],

build-system/test-configs/conformance-config.textproto

@@ -468,7 +468,7 @@ requirement: {
   value: 'module$src$cookies.getCookie'
   value: 'module$src$cookies.setCookie'
   allowlist: 'src/cookies.js'
-  allowlist: 'src/experiments.js'
+  allowlist: 'src/experiments/index.js'
   allowlist: 'src/service/cid-api.js'
   allowlist: 'src/service/cid-impl.js'
   allowlist: 'extensions/amp-analytics/0.1/cookie-writer.js'

build-system/test-configs/dep-check-config.js

@@ -138,7 +138,7 @@ exports.rules = [
       'ads/google/a4a/**->src/ad-cid.js',
       'ads/google/a4a/**->src/consent.js',
       'ads/google/a4a/**->src/dom.js',
-      'ads/google/a4a/**->src/experiments.js',
+      'ads/google/a4a/**->src/experiments/index.js',
       'ads/google/a4a/**->src/services.js',
       'ads/google/a4a/utils.js->src/service/variable-source.js',
       'ads/google/a4a/utils.js->src/ini-load.js',

build-system/test-configs/forbidden-terms.js

@@ -460,7 +460,7 @@ const forbiddenTermsGlobal = {
       'extensions/amp-story/1.0/history.js',
       'extensions/amp-web-push/0.1/amp-web-push-helper-frame.js',
       'extensions/amp-web-push/0.1/amp-web-push-permission-dialog.js',
-      'src/experiments.js',
+      'src/experiments/index.js',
       'src/service/cid-impl.js',
       'src/service/storage-impl.js',
       'testing/init-tests.js',
@@ -625,7 +625,8 @@ const forbiddenTermsGlobal = {
       'build-system/tasks/dist.js',
       'build-system/tasks/helpers.js',
       'src/config.js',
-      'src/experiments.js',
+      'src/experiments/index.js',
+      'src/experiments/shame.extern.js',
       'src/mode.js',
       'src/web-worker/web-worker.js', // Web worker custom error reporter.
       'testing/init-tests.js',

extensions/amp-analytics/0.1/test/test-analytics-group.js

@@ -26,7 +26,7 @@ import {
   VisibilityTracker,
 } from '../events';
 import {AnalyticsGroup} from '../analytics-group.js';
-import {toggleExperiment} from '../../../../src/experiments.js';
+import {toggleExperiment} from '../../../../src/experiments';
 
 describes.realWin('AnalyticsGroup', {amp: 1}, (env) => {
   let win;

src/.eslintrc.js

@@ -71,6 +71,8 @@ module.exports = {
       'files': [
         './core/window.extern.js',
         './polyfills/custom-elements.extern.js',
+        './experiments/experiments.extern.js',
+        './experiments/shame.extern.js',
       ],
       'rules': {'local/no-global': 0},
     },

src/examiner/examiner.js

@@ -18,21 +18,20 @@
  * @param {!Window} win
  */
 function detectLongTasks(win) {
-  const observer = new win.PerformanceObserver(function (entryList) {
+  const observer = new win.PerformanceObserver((entryList) => {
+    /** @type {!Array<!PerformanceLongTaskTiming>} */
     const entries = entryList.getEntries();
-    for (let i = 0; i < entries.length; i++) {
-      if (
-        entries[i].entryType != 'longtask' ||
-        entries[i].name != 'cross-origin-descendant'
-      ) {
+    for (const entry of entries) {
+      const {entryType, name, attribution, duration} = entry;
+
+      if (entryType != 'longtask' || name != 'cross-origin-descendant') {
         continue;
       }
-      const attr = entries[i].attribution[0];
-      if (!attr || !attr.containerSrc) {
+      const attr = attribution[0];
+      if (!attr?.containerSrc) {
         continue;
       }
 
-      const {duration} = entries[i];
       let culprit = attr.containerSrc;
       if (attr.containerName) {
         const match = attr.containerName.match(/"type":"([^\"]*)"/);

src/experiments/ads-initial-intersection-exp.js

@@ -14,7 +14,7 @@
  * limitations under the License.
  */
 
-/** @const {!{id: string, control: string, experiment: string}} */
+/** @const */
 export const ADS_INITIAL_INTERSECTION_EXP = {
   id: 'ads-initialIntersection',
   control: '31060065',

src/experiments/experiments.extern.js

@@ -0,0 +1,23 @@
+/**
+ * Copyright 2021 The AMP HTML 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.
+ */
+
+/**
+ * @fileoverview global externs for the experiment system.
+ * @externs
+ */
+
+/** @type {undefined|Object<string, ?string>} */
+window.__AMP_EXPERIMENT_BRANCHES;

src/experiments/experiments.type.js

@@ -0,0 +1,24 @@
+/**
+ * Copyright 2021 The AMP HTML 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.
+ */
+
+/**
+ * @typedef {{
+ *   experimentId: string,
+ *   isTrafficEligible: function(!Window):boolean,
+ *   branches: !Array<string>
+ * }}
+ */
+export let ExperimentInfoDef;

src/experiments.js → src/experiments/index.js

@@ -21,11 +21,15 @@
  * Experiments page: https://cdn.ampproject.org/experiments.html *
  */
 
-import {dev, user} from './log';
-import {getMode} from './mode';
-import {getTopWindow} from './service';
-import {hasOwn} from './core/types/object';
-import {parseQueryString} from './url';
+import {dev, user} from '../log';
+import {getMode} from '../mode';
+import {getTopWindow} from '../service';
+import {hasOwn, map} from '../core/types/object';
+import {isArray} from '../core/types';
+import {parseQueryString} from '../url';
+
+// typedef imports
+import {ExperimentInfoDef} from './experiments.type';
 
 /** @const {string} */
 const TAG = 'EXPERIMENTS';
@@ -36,22 +40,13 @@ const LOCAL_STORAGE_KEY = 'amp-experiment-toggles';
 /** @const {string} */
 const TOGGLES_WINDOW_PROPERTY = '__AMP__EXPERIMENT_TOGGLES';
 
-/**
- * @typedef {{
- *   experimentId: string,
- *   isTrafficEligible: function(!Window):boolean,
- *   branches: !Array<string>
- * }}
- */
-export let ExperimentInfo;
-
 /**
  * Whether we are in canary.
  * @param {!Window} win
  * @return {boolean}
  */
 export function isCanary(win) {
-  return !!(win.AMP_CONFIG && win.AMP_CONFIG.canary);
+  return !!win.AMP_CONFIG?.canary;
 }
 
 /**
@@ -60,9 +55,7 @@ export function isCanary(win) {
  * @return {string}
  */
 export function getBinaryType(win) {
-  return win.AMP_CONFIG && win.AMP_CONFIG.type
-    ? win.AMP_CONFIG.type
-    : 'unknown';
+  return win.AMP_CONFIG?.type || 'unknown';
 }
 
 /**
@@ -95,7 +88,7 @@ export function toggleExperiment(
   opt_transientExperiment
 ) {
   const currentlyOn = isExperimentOn(win, /*OK*/ experimentId);
-  const on = !!(opt_on !== undefined ? opt_on : !currentlyOn);
+  const on = opt_on ?? !currentlyOn;
   if (on != currentlyOn) {
     const toggles = experimentToggles(win);
     toggles[experimentId] = on;
@@ -129,7 +122,7 @@ export function experimentToggles(win) {
   if (win[TOGGLES_WINDOW_PROPERTY]) {
     return win[TOGGLES_WINDOW_PROPERTY];
   }
-  win[TOGGLES_WINDOW_PROPERTY] = Object.create(null);
+  win[TOGGLES_WINDOW_PROPERTY] = map();
   const toggles = win[TOGGLES_WINDOW_PROPERTY];
 
   // Read the default config of this build.
@@ -142,42 +135,34 @@ export function experimentToggles(win) {
     }
   }
   // Read document level override from meta tag.
-  if (
-    win.AMP_CONFIG &&
-    Array.isArray(win.AMP_CONFIG['allow-doc-opt-in']) &&
-    win.AMP_CONFIG['allow-doc-opt-in'].length > 0
-  ) {
-    const allowed = win.AMP_CONFIG['allow-doc-opt-in'];
+  const allowedDocOptIn = win.AMP_CONFIG?.['allow-doc-opt-in'];
+  if (isArray(allowedDocOptIn) && allowedDocOptIn.length) {
     const meta = win.document.head.querySelector(
       'meta[name="amp-experiments-opt-in"]'
     );
     if (meta) {
       const optedInExperiments = meta.getAttribute('content').split(',');
-      for (let i = 0; i < optedInExperiments.length; i++) {
-        if (allowed.indexOf(optedInExperiments[i]) != -1) {
-          toggles[optedInExperiments[i]] = true;
+      for (const experiment of optedInExperiments) {
+        if (dev().assertArray(allowedDocOptIn).includes(experiment)) {
+          toggles[experiment] = true;
         }
       }
     }
   }
 
   Object.assign(toggles, getExperimentToggles(win));
 
-  if (
-    win.AMP_CONFIG &&
-    Array.isArray(win.AMP_CONFIG['allow-url-opt-in']) &&
-    win.AMP_CONFIG['allow-url-opt-in'].length > 0
-  ) {
-    const allowed = win.AMP_CONFIG['allow-url-opt-in'];
+  const allowedUrlOptIn = win.AMP_CONFIG?.['allow-url-opt-in'];
+  if (isArray(allowedUrlOptIn) && allowedUrlOptIn.length) {
     const hash = win.location['originalHash'] || win.location.hash;
     const params = parseQueryString(hash);
-    for (let i = 0; i < allowed.length; i++) {
-      const param = params[`e-${allowed[i]}`];
+    for (const experiment of allowedUrlOptIn) {
+      const param = params[`e-${experiment}`];
       if (param == '1') {
-        toggles[allowed[i]] = true;
+        toggles[experiment] = true;
       }
       if (param == '0') {
-        toggles[allowed[i]] = false;
+        toggles[experiment] = false;
       }
     }
   }
@@ -188,7 +173,7 @@ export function experimentToggles(win) {
  * Returns the cached experiments toggles, or null if they have not been
  * computed yet.
  * @param {!Window} win
- * @return {Object<string, boolean>}
+ * @return {?Object<string, boolean>}
  */
 export function experimentTogglesOrNull(win) {
   return win[TOGGLES_WINDOW_PROPERTY] || null;
@@ -205,20 +190,20 @@ function getExperimentToggles(win) {
     if ('localStorage' in win) {
       experimentsString = win.localStorage.getItem(LOCAL_STORAGE_KEY);
     }
-  } catch (e) {
+  } catch {
     dev().warn(TAG, 'Failed to retrieve experiments from localStorage.');
   }
-  const tokens = experimentsString ? experimentsString.split(/\s*,\s*/g) : [];
+  const tokens = experimentsString?.split(/\s*,\s*/g) || [];
 
-  const toggles = Object.create(null);
-  for (let i = 0; i < tokens.length; i++) {
-    if (tokens[i].length == 0) {
+  const toggles = map();
+  for (const token of tokens) {
+    if (!token) {
       continue;
     }
-    if (tokens[i][0] == '-') {
-      toggles[tokens[i].substr(1)] = false;
+    if (token[0] == '-') {
+      toggles[token.substr(1)] = false;
     } else {
-      toggles[tokens[i]] = true;
+      toggles[token] = true;
     }
   }
   return toggles;
@@ -235,9 +220,7 @@ function saveExperimentToggles(win, toggles) {
     experimentIds.push((toggles[experiment] === false ? '-' : '') + experiment);
   }
   try {
-    if ('localStorage' in win) {
-      win.localStorage.setItem(LOCAL_STORAGE_KEY, experimentIds.join(','));
-    }
+    win.localStorage?.setItem(LOCAL_STORAGE_KEY, experimentIds.join(','));
   } catch (e) {
     user().error(TAG, 'Failed to save experiments to localStorage.');
   }
@@ -311,24 +294,23 @@ function selectRandomItem(arr) {
  *
  * @param {!Window} win Window context on which to save experiment
  *     selection state.
- * @param {!Array<!ExperimentInfo>} experiments  Set of experiments to
+ * @param {!Array<!ExperimentInfoDef>} experiments  Set of experiments to
  *     configure for this page load.
  * @return {!Object<string, string>} Map of experiment names to selected
  *     branches.
  */
 export function randomlySelectUnsetExperiments(win, experiments) {
   win.__AMP_EXPERIMENT_BRANCHES = win.__AMP_EXPERIMENT_BRANCHES || {};
   const selectedExperiments = {};
-  for (let i = 0; i < experiments.length; i++) {
-    const experiment = experiments[i];
+  for (const experiment of experiments) {
     const experimentName = experiment.experimentId;
     if (hasOwn(win.__AMP_EXPERIMENT_BRANCHES, experimentName)) {
       selectedExperiments[experimentName] =
         win.__AMP_EXPERIMENT_BRANCHES[experimentName];
       continue;
     }
 
-    if (!experiment.isTrafficEligible || !experiment.isTrafficEligible(win)) {
+    if (!experiment.isTrafficEligible?.(win)) {
       win.__AMP_EXPERIMENT_BRANCHES[experimentName] = null;
       continue;
     }