Skip to content

Commit

Permalink
♻️ Fully pass typechecking in #experiments + references cleanup (#37214)
Browse files Browse the repository at this point in the history
* Set up #experiments for typechecking

* Enable partial typechecking on #experiments

* Rename *-globals.d.ts files for convention/consistency

* Remove extern file

* Use reference comment tag where trivial

* Partial type-checking for #experiments

* Forbidden terms

* Lint fixes

* Split up some .d.ts files

* Revert references tags

* Include all core .d.ts files

* Fix All The Things!

* Lint fixes

* Remove unneeded assertions

* Merge in and use shared config

* Comment out local/no-duplicate-name-typedef

* Lint fixes

* Un-rename accidental rename

* Fix @ const {string} for prop literals

* Fix remaining type errors after removing allowance

* Remove unneeded annotations on FORM_PROP_
  • Loading branch information
rcebulko committed Dec 16, 2021
1 parent ef76929 commit c20155c
Show file tree
Hide file tree
Showing 31 changed files with 154 additions and 106 deletions.
1 change: 0 additions & 1 deletion ads/google/a4a/experiment-utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import {
getExperimentBranch,
randomlySelectUnsetExperiments,
} from '#experiments';
import {ExperimentInfoDef} from '#experiments/experiments.type';

/**
* Attempts to select into experiment and forces branch if selected.
Expand Down
1 change: 1 addition & 0 deletions build-system/tasks/check-types.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ const TSC_TYPECHECK_TARGETS = {
'compiler': 'src/compiler',
'carousel': 'extensions/amp-carousel/0.1',
'core': 'src/core',
'experiments': 'src/experiments',
};

/**
Expand Down
5 changes: 1 addition & 4 deletions build-system/test-configs/forbidden-terms.js
Original file line number Diff line number Diff line change
Expand Up @@ -607,10 +607,7 @@ const forbiddenTermsGlobal = {
'\\b(__)?AMP_EXP\\b': {
message:
'Do not access AMP_EXP directly. Use isExperimentOn() to access config',
allowlist: [
'src/experiments/index.js',
'src/experiments/experiments.extern.js',
],
allowlist: ['src/experiments/index.js'],
},
'AMP_CONFIG': {
message:
Expand Down
3 changes: 0 additions & 3 deletions src/.eslintrc.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@

const {isCiBuild} = require('../build-system/common/ci');

module.exports = {
Expand Down Expand Up @@ -54,9 +53,7 @@ module.exports = {
},
{
'files': [
'./core/window/window.extern.js',
'./polyfills/custom-elements.extern.js',
'./experiments/experiments.extern.js',
'./experiments/shame.extern.js',
],
'rules': {'local/no-global': 0},
Expand Down
9 changes: 8 additions & 1 deletion src/core/amp-config.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,15 @@ export {};

declare global {
// AMP configuration and runtime settings structure.
interface AmpConfigDef {
interface AmpConfig {
test?: boolean;
localDev?: boolean;
canary?: boolean;
type?: string;
}

interface Window {
// AMP Runtime settings, configuration, and environment/build constants.
AMP_CONFIG?: AmpConfig & {[key: string]: (boolean|string)};
}
}
12 changes: 12 additions & 0 deletions src/core/amp-mode.d.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
export {};

declare global {
interface AmpModeObject {
esm: boolean;
}

interface Window {
// AMP Mode, used to force an override in tests.
__AMP_MODE: AmpModeObject;
}
}
11 changes: 11 additions & 0 deletions src/core/assert/assert-globals.d.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
export {};

declare global {
interface Window {
// Never exists; used as part of post-compilation checks to verify DCE.
__AMP_ASSERTION_CHECK: undefined;

// Global error reporting handler; only present in AMP pages.
__AMP_REPORT_ERROR?: (this: Window, err: Error, opt_el?: Element) => void;
}
}
2 changes: 1 addition & 1 deletion src/core/assert/error.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,6 @@ export {};

declare global {
interface Error {
messageArray?: Array<any>;
messageArray?: any[];
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -3,5 +3,8 @@ export {};
declare global {
interface Window {
WeakRef?: typeof WeakRef;

// Counter for the DomBaseWeakRef polyfill.
__AMP_WEAKREF_ID?: number;
}
}
3 changes: 0 additions & 3 deletions src/core/dom/amp-element-helpers.js
Original file line number Diff line number Diff line change
@@ -1,10 +1,7 @@
import {devAssert} from '#core/assert';
import {Deferred} from '#core/data-structures/promise';

/** @const {string} */
export const UPGRADE_TO_CUSTOMELEMENT_PROMISE = '__AMP_UPG_PRM';

/** @const {string} */
export const UPGRADE_TO_CUSTOMELEMENT_RESOLVER = '__AMP_UPG_RES';

/**
Expand Down
1 change: 0 additions & 1 deletion src/core/dom/form.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ import {ancestorElementsByTag} from '#core/dom/query';
import {toArray} from '#core/types/array';
import {dict} from '#core/types/object';

/** @const {string} */
const FORM_PROP_ = '__AMP_FORM';

/**
Expand Down
File renamed without changes.
4 changes: 0 additions & 4 deletions src/core/error/message-helpers.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,14 +6,10 @@ import {isElement} from '#core/types';
* This is added to user error messages, so that we can later identify
* them, when the only thing that we have is the message. This is the
* case in many browsers when the global exception handler is invoked.
*
* @const {string}
*/
export const USER_ERROR_SENTINEL = '\u200B\u200B\u200B';
/**
* Four zero width space.
*
* @const {string}
*/
export const USER_ERROR_EMBED_SENTINEL = '\u200B\u200B\u200B\u200B';

Expand Down
File renamed without changes.
2 changes: 1 addition & 1 deletion src/core/tsconfig.json
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
{
"compilerOptions": {"baseUrl": "../.."},
"extends": "../../tsconfig.base.json",
"compilerOptions": {"baseUrl": "../.."},
"include": ["**/*.js", "**/*.d.ts"]
}
File renamed without changes.
15 changes: 0 additions & 15 deletions src/core/window/window.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,25 +2,10 @@ export {};

declare global {
interface Window {
// Never exists; used as part of post-compilation checks to verify DCE.
__AMP_ASSERTION_CHECK: undefined;

// Global error reporting handler; only present in AMP pages.
__AMP_REPORT_ERROR?: (this: Window, err: Error, opt_el?: Element) => void;

// Global property set by test some harnesses to signal a testing environment.
__AMP_TEST?: boolean;

// Global property set by test some harnesses to signal karma testing environment.
__karma__?: boolean;

// Counter for the DomBaseWeakRef polyfill.
__AMP_WEAKREF_ID?: number;

// AMP Runtime settings, configuration, and environment/build constants.
AMP_CONFIG?: AmpConfigDef;

// AMP Mode, used to force an override in tests.
__AMP_MODE: {esm: boolean};
}
}
10 changes: 10 additions & 0 deletions src/experiments/experiments-globals.d.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
export {};

declare global {
interface Window {
__AMP_EXPERIMENT_BRANCHES?: import('./types.d').ExperimentBranchMap;
__AMP__EXPERIMENT_TOGGLES?: {[key: string]: boolean};
__AMP_EXP?: HTMLScriptElement;
AMP_EXP?: {[key: string]: number};
}
}
12 changes: 0 additions & 12 deletions src/experiments/experiments.extern.js

This file was deleted.

8 changes: 0 additions & 8 deletions src/experiments/experiments.type.js

This file was deleted.

0 comments on commit c20155c

Please sign in to comment.