Specify ambient type for lodash

[samreid]

WebStorm indicates a warning TS2686 on all usages of lodash code. I worked with @jessegreenberg to change the inspections, disable the inspections, set compiler options to allow and set command line options to allow, but WebStorm was intransigent. Instead, we found a way to import the types from the @types declaration and re-export as a global. This has to be done in a different file, since the import wrecks the other ambient declare var exports. But this seems to be working great in my IDE. Here's a patch. I'll run it past @jessegreenberg to see if it works well on his side.

[samreid]

OK here's the patch: @jessegreenberg seem OK on your side?

````diff Subject: [PATCH] hello --- Index: main/phet-info/doc/typescript-quick-start.md IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/main/phet-info/doc/typescript-quick-start.md b/main/phet-info/doc/typescript-quick-start.md --- a/main/phet-info/doc/typescript-quick-start.md (revision 4e3fab4f609200652378141160bd78e0c5169c81) +++ b/main/phet-info/doc/typescript-quick-start.md (date 1691767305581) @@ -33,6 +33,7 @@ ```json "../chipper/phet-types.d.ts", +"../chipper/phet-types-via-imports.d.ts", "../chipper/node_modules/@types/lodash/index.d.ts", "../chipper/node_modules/@types/qunit/index.d.ts" ``` @@ -82,7 +83,7 @@ 2. phettest and CT provide TypeScript support, but do not yet have a good user experience for showing TypeErrors etc. And it is not well-vetted. 3. Please make sure you are using the commit hooks, which are configured to run type checks on typescript repos. -4. Ambient type definitions are provided in chipper/phet-types.d.ts +4. Ambient type definitions are provided in chipper/phet-types.d.ts and phet-types-via-imports.d.ts 5. Transitive dependencies are not always tracked correctly in the build system. This bug has been reported to TypeScript. Details in https://github.com/phetsims/chipper/issues/1067 6. Some common code repos include code outside their directory. This problem is described Index: main/chipper/tsconfig-core.json IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/main/chipper/tsconfig-core.json b/main/chipper/tsconfig-core.json --- a/main/chipper/tsconfig-core.json (revision c0e48032d153ce90a2db6cb21946def72deb0c37) +++ b/main/chipper/tsconfig-core.json (date 1691767305587) @@ -76,6 +76,7 @@ }, "files": [ "phet-types.d.ts", + "phet-types-via-imports.d.ts", "node_modules/@types/jquery/index.d.ts", "node_modules/@types/lodash/index.d.ts", "node_modules/@types/p2/index.d.ts", Index: main/chipper/phet-types-via-imports.d.ts IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/main/chipper/phet-types-via-imports.d.ts b/main/chipper/phet-types-via-imports.d.ts new file mode 100644 --- /dev/null (date 1691767524163) +++ b/main/chipper/phet-types-via-imports.d.ts (date 1691767524163) @@ -0,0 +1,20 @@ +// Copyright 2021, University of Colorado Boulder + +/** + * Ambient type declarations for PhET code that require import statements. + * + * @author Sam Reid (PhET Interactive Simulations) + */ + +import * as lodash from '../../../chipper/node_modules/@types/lodash/index.js'; + +declare global { + + // the types provided by @types/lodash assume that you're importing lodash using ES6-style imports or require statements. + // + // By creating a global augmentation for lodash (i.e., re-declaring it as a global variable in the ambient context), + // you're bridging the gap between the runtime behavior (lodash is a global variable) and TypeScript's type checking + // (lodash should be imported). Without this global declaration, TypeScript would complain whenever you try to use + // lodash without an import statement because it doesn't see _ as a global variable by default. + const _: typeof lodash; +} \ No newline at end of file Index: main/chipper/tsconfig/all/tsconfig.json IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/main/chipper/tsconfig/all/tsconfig.json b/main/chipper/tsconfig/all/tsconfig.json --- a/main/chipper/tsconfig/all/tsconfig.json (revision c0e48032d153ce90a2db6cb21946def72deb0c37) +++ b/main/chipper/tsconfig/all/tsconfig.json (date 1691767331679) @@ -20,6 +20,7 @@ "../../../center-and-variability/js/**/*", "../../../chipper/js/phet-build-script/phet-build-script.ts", "../../../chipper/phet-types.d.ts", + "../../../chipper/phet-types-via-imports.d.ts", "../../../circuit-construction-kit-ac/js/**/*", "../../../circuit-construction-kit-ac-virtual-lab/js/**/*", "../../../circuit-construction-kit-common/js/**/*", ````

[samreid]

That patch has other awkward behavior, unassigning @jessegreenberg for now.

[samreid]

Can't disable typescript inspection on the JetBrain forum: https://intellij-support.jetbrains.com/hc/en-us/community/posts/360003428920-Can-t-disable-typescript-inspection

[samreid]

I recreated the patch above and it actually seems to be working great.

````diff Subject: [PATCH] If there is a focused soccer ball, i.e. a soccer ball that has been selected or tabbed to via the keyboard, that takes precedence for indication, see https://github.com/phetsims/center-and-variability/issues/521 --- Index: phet-info/doc/typescript-quick-start.md IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/phet-info/doc/typescript-quick-start.md b/phet-info/doc/typescript-quick-start.md --- a/phet-info/doc/typescript-quick-start.md (revision b695e012b7cacf216754d867a8d09c80f27e13cf) +++ b/phet-info/doc/typescript-quick-start.md (date 1693432171330) @@ -33,6 +33,7 @@ ```json "../chipper/phet-types.d.ts", +"../chipper/phet-types-via-imports.d.ts", "../chipper/node_modules/@types/lodash/index.d.ts", "../chipper/node_modules/@types/qunit/index.d.ts" ``` @@ -82,7 +83,7 @@ 2. phettest and CT provide TypeScript support, but do not yet have a good user experience for showing TypeErrors etc. And it is not well-vetted. 3. Please make sure you are using the commit hooks, which are configured to run type checks on typescript repos. -4. Ambient type definitions are provided in chipper/phet-types.d.ts +4. Ambient type definitions are provided in chipper/phet-types.d.ts and phet-types-via-imports.d.ts 5. Transitive dependencies are not always tracked correctly in the build system. This bug has been reported to TypeScript. Details in https://github.com/phetsims/chipper/issues/1067 6. Some common code repos include code outside their directory. This problem is described Index: chipper/phet-types-via-imports.d.ts IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/chipper/phet-types-via-imports.d.ts b/chipper/phet-types-via-imports.d.ts new file mode 100644 --- /dev/null (date 1693432171326) +++ b/chipper/phet-types-via-imports.d.ts (date 1693432171326) @@ -0,0 +1,20 @@ +// Copyright 2021, University of Colorado Boulder + +/** + * Ambient type declarations for PhET code that require import statements. + * + * @author Sam Reid (PhET Interactive Simulations) + */ + +import * as lodash from '../../../chipper/node_modules/@types/lodash/index.js'; + +declare global { + + // the types provided by @types/lodash assume that you're importing lodash using ES6-style imports or require statements. + // + // By creating a global augmentation for lodash (i.e., re-declaring it as a global variable in the ambient context), + // you're bridging the gap between the runtime behavior (lodash is a global variable) and TypeScript's type checking + // (lodash should be imported). Without this global declaration, TypeScript would complain whenever you try to use + // lodash without an import statement because it doesn't see _ as a global variable by default. + const _: typeof lodash; +} \ No newline at end of file Index: chipper/tsconfig-core.json IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/chipper/tsconfig-core.json b/chipper/tsconfig-core.json --- a/chipper/tsconfig-core.json (revision dfb884868a0c4e8b3af38b43aa2ee57814a0ad25) +++ b/chipper/tsconfig-core.json (date 1693432171323) @@ -76,6 +76,7 @@ }, "files": [ "phet-types.d.ts", + "phet-types-via-imports.d.ts", "node_modules/@types/jquery/index.d.ts", "node_modules/@types/lodash/index.d.ts", "node_modules/@types/p2/index.d.ts", Index: chipper/tsconfig/all/tsconfig.json IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/chipper/tsconfig/all/tsconfig.json b/chipper/tsconfig/all/tsconfig.json --- a/chipper/tsconfig/all/tsconfig.json (revision dfb884868a0c4e8b3af38b43aa2ee57814a0ad25) +++ b/chipper/tsconfig/all/tsconfig.json (date 1693432171319) @@ -20,6 +20,7 @@ "../../../center-and-variability/js/**/*", "../../../chipper/js/phet-build-script/phet-build-script.ts", "../../../chipper/phet-types.d.ts", + "../../../chipper/phet-types-via-imports.d.ts", "../../../circuit-construction-kit-ac/js/**/*", "../../../circuit-construction-kit-ac-virtual-lab/js/**/*", "../../../circuit-construction-kit-common/js/**/*", ````

Still, I'd like to consult with someone before committing.

[samreid]

@zepumph and I reproduced all those tests on his side, including testing --allowUmdGlobalAccess in the typescript command line options in WebStorm. On @zepumph setup, we had all the same problems.

Then when applying the patch, it seemed all the problems were resolved. So @zepumph and I agreed it seems ok to commit the patch. We also had an idea that we could merge the phet-types and phet-types-with imports if we change the way the exports work in that file. I'll try that and commit if it works on my side.

[samreid]

Our codebase is a mishmash of window.myGlobal and sometimes just myGlobal. When I merged the declaration files, it brought this distinction to light, because it no longer supported both variations (why?). Here's my untested patch so far, still have around 20 errors left to go:

```diff Subject: [PATCH] If there is a focused soccer ball, i.e. a soccer ball that has been selected or tabbed to via the keyboard, that takes precedence for indication, see https://github.com/phetsims/center-and-variability/issues/521 --- Index: chipper/phet-types.d.ts IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/chipper/phet-types.d.ts b/chipper/phet-types.d.ts --- a/chipper/phet-types.d.ts (revision dfb884868a0c4e8b3af38b43aa2ee57814a0ad25) +++ b/chipper/phet-types.d.ts (date 1693436852420) @@ -1,7 +1,5 @@ // Copyright 2021, University of Colorado Boulder -/* eslint-disable no-var */ - /** * Ambient type declarations for PhET code. Many of these definitions can be moved/disabled once the common code is * converted to TypeScript. @@ -9,17 +7,9 @@ * @author Sam Reid (PhET Interactive Simulations) */ -// Can't externally reference -type IntentionalAny = any; // eslint-disable-line @typescript-eslint/no-explicit-any - -declare var assert: undefined | ( ( x: IntentionalAny, ...messages?: IntentionalAny[] ) => void ); -declare var assertSlow: undefined | ( ( x: IntentionalAny, ...messages?: IntentionalAny[] ) => void ); -declare var sceneryLog: null | false | ( Record void> & { - push(): void; - pop(): void; - getDepth(): number; -} ); -declare var phet: Record; +// eslint-disable-next-line bad-sim-text +import * as lodash from 'lodash'; +import IntentionalAny from '../phet-core/js/types/IntentionalAny.js'; // TODO: This can be moved to QueryStringMachine when it is moved to TypeScript, see https://github.com/phetsims/query-string-machine/issues/49 declare type Warning = { @@ -70,64 +60,83 @@ // Converts a Schema's type to the actual Typescript type it represents declare type QueryMachineTypeToType = T extends ( 'flag' | 'boolean' ) ? boolean : ( T extends 'number' ? number : ( T extends 'string' ? ( string | null ) : ( T extends 'array' ? IntentionalAny[] : IntentionalAny ) ) ); -declare var QueryStringMachine: { - getAll: >( a: SchemaMap ) => { - // Will return a map of the "result" types - [Property in keyof SchemaMap]: QueryMachineTypeToType - // SCHEMA_MAP allowed to be set in types - } & { SCHEMA_MAP?: Record }; - getAllForString: >( a: SchemaMap, b: string ) => { - // Will return a map of the "result" types - [Property in keyof SchemaMap]: QueryMachineTypeToType - // SCHEMA_MAP allowed to be set in types - } & { SCHEMA_MAP?: Record }; - get: ( a: string, schema: Schema ) => QueryMachineTypeToType; - containsKey: ( key: string ) => boolean; - warnings: Warning[]; - addWarning: ( key: string, value: IntentionalAny, message: string ) => void; - removeKeyValuePair: ( queryString: string, key: string ) => string; - removeKeyValuePairs: ( queryString: string, keys: string[] ) => string; - appendQueryString: ( url: string, tail: string ) => string; - getForString: ( s: string, schema: QueryStringMachineSchema, s: string ) => string; - getQueryString: ( url: string ) => string; - containsKeyForString: ( key: string, s: string ) => boolean; - getSingleQueryParameterString: ( key: string, url: string ) => string | null; - getQueryParametersFromString: ( string: string ) => string[]; -}; +// Typing for linebreaker-1.1.0.js preload +declare type LineBreakerBreak = { + position: number; + required: boolean; +}; +declare type LineBreakerType = { + nextBreak(): LineBreakerBreak | null; + + // We make it iterable + [ Symbol.iterator ](): Iterator; +}; + +declare global { + + // the types provided by @types/lodash assume that you're importing lodash using ES6-style imports or require statements. + // + // By creating a global augmentation for lodash (i.e., re-declaring it as a global variable in the ambient context), + // you're bridging the gap between the runtime behavior (lodash is a global variable) and TypeScript's type checking + // (lodash should be imported). Without this global declaration, TypeScript would complain whenever you try to use + // lodash without an import statement because it doesn't see _ as a global variable by default. + const _: typeof lodash; + + const assert: undefined | ( ( x: IntentionalAny, ...messages?: IntentionalAny[] ) => void ); + const assertSlow: undefined | ( ( x: IntentionalAny, ...messages?: IntentionalAny[] ) => void ); + const sceneryLog: null | false | ( Record void> & { + push(): void; + pop(): void; + getDepth(): number; + } ); + const phet: Record; + + const QueryStringMachine: { + getAll: >( a: SchemaMap ) => { + // Will return a map of the "result" types + [Property in keyof SchemaMap]: QueryMachineTypeToType + // SCHEMA_MAP allowed to be set in types + } & { SCHEMA_MAP?: Record }; + getAllForString: >( a: SchemaMap, b: string ) => { + // Will return a map of the "result" types + [Property in keyof SchemaMap]: QueryMachineTypeToType + // SCHEMA_MAP allowed to be set in types + } & { SCHEMA_MAP?: Record }; + get: ( a: string, schema: Schema ) => QueryMachineTypeToType; + containsKey: ( key: string ) => boolean; + warnings: Warning[]; + addWarning: ( key: string, value: IntentionalAny, message: string ) => void; + removeKeyValuePair: ( queryString: string, key: string ) => string; + removeKeyValuePairs: ( queryString: string, keys: string[] ) => string; + appendQueryString: ( url: string, tail: string ) => string; + getForString: ( s: string, schema: QueryStringMachineSchema, s: string ) => string; + getQueryString: ( url: string ) => string; + containsKeyForString: ( key: string, s: string ) => boolean; + getSingleQueryParameterString: ( key: string, url: string ) => string | null; + getQueryParametersFromString: ( string: string ) => string[]; + }; -// globals used in Sim.ts -declare var phetSplashScreenDownloadComplete: () => void; -declare var TWEEN: { update: ( dt: number ) => void }; -declare var phetSplashScreen: { dispose: () => void }; -declare var phetio: Record; - -// Typing for linebreaker-1.1.0.js preload -declare type LineBreakerBreak = { - position: number; - required: boolean; -}; -declare type LineBreakerType = { - nextBreak(): LineBreakerBreak | null; - - // We make it iterable - [ Symbol.iterator ](): Iterator; -}; -declare var LineBreaker: { - new( str: string ): LineBreakerType; -}; + const LineBreaker: { + new( str: string ): LineBreakerType; + }; -declare var assertions: { - enableAssert: () => void; -}; + const Deno: { + readTextFileSync: ( file: string ) => string; + }; -// Experiment to allow accessing these off window. See https://stackoverflow.com/questions/12709074/how-do-you-explicitly-set-a-new-property-on-window-in-typescript -declare global { + // Experiment to allow accessing these off window. See https://stackoverflow.com/questions/12709074/how-do-you-explicitly-set-a-new-property-on-window-in-typescript interface Window { // eslint-disable-line @typescript-eslint/consistent-type-definitions phet: typeof phet; - phetio: typeof phetio; + assert: undefined | ( ( x: IntentionalAny, ...messages?: IntentionalAny[] ) => void ); + assertSlow: undefined | ( ( x: IntentionalAny, ...messages?: IntentionalAny[] ) => void ); + + // globals used in Sim.ts + phetSplashScreenDownloadComplete: () => void; + TWEEN: { update: ( dt: number ) => void }; + phetSplashScreen: { dispose: () => void }; + phetio: Record; + assertions: { + enableAssert: () => void; + }; } -} - -declare var Deno: { - readTextFileSync: ( file: string ) => string; -}; \ No newline at end of file +} \ No newline at end of file Index: studio/js/Search.ts IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/studio/js/Search.ts b/studio/js/Search.ts --- a/studio/js/Search.ts (revision b28fbbf744437836771281f0b850933d50a3aaf7) +++ b/studio/js/Search.ts (date 1693436898734) @@ -114,11 +114,9 @@ // While searching, prohibit any sort of focus behavior from triggering in the sim, see https://github.com/phetsims/studio/issues/252 searchBar.addEventListener( 'focusin', () => { - // @ts-expect-error simFrame.contentWindow!.phet.joist.sim.display.simulationRoot.pdomVisible = false; } ); searchBar.addEventListener( 'focusout', () => { - // @ts-expect-error simFrame.contentWindow!.phet.joist.sim.display.simulationRoot.pdomVisible = true; } ); } @@ -292,7 +290,7 @@ let matchArray = nonEndingMatches; // separate storage for when the phetioID's component name matches the search - if ( phetio.PhetioIDUtils.getComponentName( treeNode.phetioID ).toLowerCase().includes( searchTerm.toLowerCase() ) ) { + if ( window.phetio.PhetioIDUtils.getComponentName( treeNode.phetioID ).toLowerCase().includes( searchTerm.toLowerCase() ) ) { matchArray = endingMatches; } matchArray.push( treeNode ); Index: models-of-the-hydrogen-atom/js/common/MOTHAQueryParameters.ts IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/models-of-the-hydrogen-atom/js/common/MOTHAQueryParameters.ts b/models-of-the-hydrogen-atom/js/common/MOTHAQueryParameters.ts --- a/models-of-the-hydrogen-atom/js/common/MOTHAQueryParameters.ts (revision 446cd03289dde9709ae9e4472f259c8f5021a881) +++ b/models-of-the-hydrogen-atom/js/common/MOTHAQueryParameters.ts (date 1693436707158) @@ -9,6 +9,7 @@ import logGlobal from '../../../phet-core/js/logGlobal.js'; import modelsOfTheHydrogenAtom from '../modelsOfTheHydrogenAtom.js'; +import { QueryStringMachineSchema } from '../../../chipper/phet-types.js'; const SCHEMA_MAP: Record = { Index: tandem/js/PhetioDynamicElementContainer.ts IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/tandem/js/PhetioDynamicElementContainer.ts b/tandem/js/PhetioDynamicElementContainer.ts --- a/tandem/js/PhetioDynamicElementContainer.ts (revision 10b19cd8579375a495507000287d041bd2c4395f) +++ b/tandem/js/PhetioDynamicElementContainer.ts (date 1693436518206) @@ -236,7 +236,7 @@ private stateSetOnAllChildrenOfDynamicElement( dynamicElementID: string, stillToSetIDs: string[] ): boolean { for ( let i = 0; i < stillToSetIDs.length; i++ ) { - if ( phetio.PhetioIDUtils.isAncestor( dynamicElementID, stillToSetIDs[ i ] ) ) { + if ( window.phetio.PhetioIDUtils.isAncestor( dynamicElementID, stillToSetIDs[ i ] ) ) { return false; } } Index: tandem/js/PhetioObject.ts IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/tandem/js/PhetioObject.ts b/tandem/js/PhetioObject.ts --- a/tandem/js/PhetioObject.ts (revision 10b19cd8579375a495507000287d041bd2c4395f) +++ b/tandem/js/PhetioObject.ts (date 1693436606286) @@ -746,7 +746,7 @@ const children = Object.keys( this.tandem.children ); const linkedChildren: LinkedElement[] = []; children.forEach( childName => { - const childPhetioID = phetio.PhetioIDUtils.append( this.tandem.phetioID, childName ); + const childPhetioID = window.phetio.PhetioIDUtils.append( this.tandem.phetioID, childName ); // Note that if it doesn't find a phetioID, that may be a synthetic node with children but not itself instrumented. const phetioObject: PhetioObject | undefined = phet.phetio.phetioEngine.phetioObjectMap[ childPhetioID ]; @@ -755,7 +755,7 @@ } } ); const linkedTandemNames = linkedChildren.map( ( linkedElement: LinkedElement ): string => { - return phetio.PhetioIDUtils.getComponentName( linkedElement.phetioID ); + return window.phetio.PhetioIDUtils.getComponentName( linkedElement.phetioID ); } ); let linkedChild: LinkedElement | null = null; if ( linkedChildren.length === 1 ) { Index: joist/js/QueryParametersWarningDialog.ts IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/joist/js/QueryParametersWarningDialog.ts b/joist/js/QueryParametersWarningDialog.ts --- a/joist/js/QueryParametersWarningDialog.ts (revision 3c8f2717688ca16889543fd2cc2766c59eec743a) +++ b/joist/js/QueryParametersWarningDialog.ts (date 1693436370327) @@ -13,6 +13,7 @@ import { Text } from '../../scenery/js/imports.js'; import joist from './joist.js'; import JoistStrings from './JoistStrings.js'; +import { Warning } from '../../chipper/phet-types.js'; type SelfOptions = EmptySelfOptions; export type QueryParametersWarningDialogOptions = SelfOptions & OopsDialogOptions; Index: scenery/js/nodes/RichText.ts IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/scenery/js/nodes/RichText.ts b/scenery/js/nodes/RichText.ts --- a/scenery/js/nodes/RichText.ts (revision 0f7341a2e6e257b640466fa418d28be4c180e7e8) +++ b/scenery/js/nodes/RichText.ts (date 1693436550296) @@ -109,7 +109,7 @@ export type RichTextLinks = RichTextLinksObject | true; // Used only for guarding against assertions, we want to know that we aren't in stringTesting mode -const isStringTest = window.QueryStringMachine && QueryStringMachine.containsKey( 'stringTest' ); +const isStringTest = window.hasOwnProperty( 'QueryStringMachine' ) && QueryStringMachine.containsKey( 'stringTest' ); type SelfOptions = { // Sets how bounds are determined for text Index: natural-selection/js/common/model/parseInitialPopulation.ts IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/natural-selection/js/common/model/parseInitialPopulation.ts b/natural-selection/js/common/model/parseInitialPopulation.ts --- a/natural-selection/js/common/model/parseInitialPopulation.ts (revision 90feec8a1f6feefe52c608e8b69c2efde8331d4c) +++ b/natural-selection/js/common/model/parseInitialPopulation.ts (date 1693436750462) @@ -27,6 +27,7 @@ import BunnyVariety from './BunnyVariety.js'; import Gene from './Gene.js'; import GenePool from './GenePool.js'; +import IntentionalAny from '../../../../phet-core/js/types/IntentionalAny.js'; type AllelesPair = { fatherAllele: Allele | null; ```

[samreid]

Also, does using the "module" mode mean types declarations need to be imported at call sites? I suspect so. That would be a change, but probably for the better. Here is my current patch, probably the same as the patch in the prior comment, but I need to shelve it now to work on other things.

```diff Subject: [PATCH] If there is a focused soccer ball, i.e. a soccer ball that has been selected or tabbed to via the keyboard, that takes precedence for indication, see https://github.com/phetsims/center-and-variability/issues/521 --- Index: chipper/phet-types.d.ts IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/chipper/phet-types.d.ts b/chipper/phet-types.d.ts --- a/chipper/phet-types.d.ts (revision dfb884868a0c4e8b3af38b43aa2ee57814a0ad25) +++ b/chipper/phet-types.d.ts (date 1693436852420) @@ -1,7 +1,5 @@ // Copyright 2021, University of Colorado Boulder -/* eslint-disable no-var */ - /** * Ambient type declarations for PhET code. Many of these definitions can be moved/disabled once the common code is * converted to TypeScript. @@ -9,17 +7,9 @@ * @author Sam Reid (PhET Interactive Simulations) */ -// Can't externally reference -type IntentionalAny = any; // eslint-disable-line @typescript-eslint/no-explicit-any - -declare var assert: undefined | ( ( x: IntentionalAny, ...messages?: IntentionalAny[] ) => void ); -declare var assertSlow: undefined | ( ( x: IntentionalAny, ...messages?: IntentionalAny[] ) => void ); -declare var sceneryLog: null | false | ( Record void> & { - push(): void; - pop(): void; - getDepth(): number; -} ); -declare var phet: Record; +// eslint-disable-next-line bad-sim-text +import * as lodash from 'lodash'; +import IntentionalAny from '../phet-core/js/types/IntentionalAny.js'; // TODO: This can be moved to QueryStringMachine when it is moved to TypeScript, see https://github.com/phetsims/query-string-machine/issues/49 declare type Warning = { @@ -70,64 +60,83 @@ // Converts a Schema's type to the actual Typescript type it represents declare type QueryMachineTypeToType = T extends ( 'flag' | 'boolean' ) ? boolean : ( T extends 'number' ? number : ( T extends 'string' ? ( string | null ) : ( T extends 'array' ? IntentionalAny[] : IntentionalAny ) ) ); -declare var QueryStringMachine: { - getAll: >( a: SchemaMap ) => { - // Will return a map of the "result" types - [Property in keyof SchemaMap]: QueryMachineTypeToType - // SCHEMA_MAP allowed to be set in types - } & { SCHEMA_MAP?: Record }; - getAllForString: >( a: SchemaMap, b: string ) => { - // Will return a map of the "result" types - [Property in keyof SchemaMap]: QueryMachineTypeToType - // SCHEMA_MAP allowed to be set in types - } & { SCHEMA_MAP?: Record }; - get: ( a: string, schema: Schema ) => QueryMachineTypeToType; - containsKey: ( key: string ) => boolean; - warnings: Warning[]; - addWarning: ( key: string, value: IntentionalAny, message: string ) => void; - removeKeyValuePair: ( queryString: string, key: string ) => string; - removeKeyValuePairs: ( queryString: string, keys: string[] ) => string; - appendQueryString: ( url: string, tail: string ) => string; - getForString: ( s: string, schema: QueryStringMachineSchema, s: string ) => string; - getQueryString: ( url: string ) => string; - containsKeyForString: ( key: string, s: string ) => boolean; - getSingleQueryParameterString: ( key: string, url: string ) => string | null; - getQueryParametersFromString: ( string: string ) => string[]; -}; +// Typing for linebreaker-1.1.0.js preload +declare type LineBreakerBreak = { + position: number; + required: boolean; +}; +declare type LineBreakerType = { + nextBreak(): LineBreakerBreak | null; + + // We make it iterable + [ Symbol.iterator ](): Iterator; +}; + +declare global { + + // the types provided by @types/lodash assume that you're importing lodash using ES6-style imports or require statements. + // + // By creating a global augmentation for lodash (i.e., re-declaring it as a global variable in the ambient context), + // you're bridging the gap between the runtime behavior (lodash is a global variable) and TypeScript's type checking + // (lodash should be imported). Without this global declaration, TypeScript would complain whenever you try to use + // lodash without an import statement because it doesn't see _ as a global variable by default. + const _: typeof lodash; + + const assert: undefined | ( ( x: IntentionalAny, ...messages?: IntentionalAny[] ) => void ); + const assertSlow: undefined | ( ( x: IntentionalAny, ...messages?: IntentionalAny[] ) => void ); + const sceneryLog: null | false | ( Record void> & { + push(): void; + pop(): void; + getDepth(): number; + } ); + const phet: Record; + + const QueryStringMachine: { + getAll: >( a: SchemaMap ) => { + // Will return a map of the "result" types + [Property in keyof SchemaMap]: QueryMachineTypeToType + // SCHEMA_MAP allowed to be set in types + } & { SCHEMA_MAP?: Record }; + getAllForString: >( a: SchemaMap, b: string ) => { + // Will return a map of the "result" types + [Property in keyof SchemaMap]: QueryMachineTypeToType + // SCHEMA_MAP allowed to be set in types + } & { SCHEMA_MAP?: Record }; + get: ( a: string, schema: Schema ) => QueryMachineTypeToType; + containsKey: ( key: string ) => boolean; + warnings: Warning[]; + addWarning: ( key: string, value: IntentionalAny, message: string ) => void; + removeKeyValuePair: ( queryString: string, key: string ) => string; + removeKeyValuePairs: ( queryString: string, keys: string[] ) => string; + appendQueryString: ( url: string, tail: string ) => string; + getForString: ( s: string, schema: QueryStringMachineSchema, s: string ) => string; + getQueryString: ( url: string ) => string; + containsKeyForString: ( key: string, s: string ) => boolean; + getSingleQueryParameterString: ( key: string, url: string ) => string | null; + getQueryParametersFromString: ( string: string ) => string[]; + }; -// globals used in Sim.ts -declare var phetSplashScreenDownloadComplete: () => void; -declare var TWEEN: { update: ( dt: number ) => void }; -declare var phetSplashScreen: { dispose: () => void }; -declare var phetio: Record; - -// Typing for linebreaker-1.1.0.js preload -declare type LineBreakerBreak = { - position: number; - required: boolean; -}; -declare type LineBreakerType = { - nextBreak(): LineBreakerBreak | null; - - // We make it iterable - [ Symbol.iterator ](): Iterator; -}; -declare var LineBreaker: { - new( str: string ): LineBreakerType; -}; + const LineBreaker: { + new( str: string ): LineBreakerType; + }; -declare var assertions: { - enableAssert: () => void; -}; + const Deno: { + readTextFileSync: ( file: string ) => string; + }; -// Experiment to allow accessing these off window. See https://stackoverflow.com/questions/12709074/how-do-you-explicitly-set-a-new-property-on-window-in-typescript -declare global { + // Experiment to allow accessing these off window. See https://stackoverflow.com/questions/12709074/how-do-you-explicitly-set-a-new-property-on-window-in-typescript interface Window { // eslint-disable-line @typescript-eslint/consistent-type-definitions phet: typeof phet; - phetio: typeof phetio; + assert: undefined | ( ( x: IntentionalAny, ...messages?: IntentionalAny[] ) => void ); + assertSlow: undefined | ( ( x: IntentionalAny, ...messages?: IntentionalAny[] ) => void ); + + // globals used in Sim.ts + phetSplashScreenDownloadComplete: () => void; + TWEEN: { update: ( dt: number ) => void }; + phetSplashScreen: { dispose: () => void }; + phetio: Record; + assertions: { + enableAssert: () => void; + }; } -} - -declare var Deno: { - readTextFileSync: ( file: string ) => string; -}; \ No newline at end of file +} \ No newline at end of file Index: studio/js/Search.ts IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/studio/js/Search.ts b/studio/js/Search.ts --- a/studio/js/Search.ts (revision b28fbbf744437836771281f0b850933d50a3aaf7) +++ b/studio/js/Search.ts (date 1693436898734) @@ -114,11 +114,9 @@ // While searching, prohibit any sort of focus behavior from triggering in the sim, see https://github.com/phetsims/studio/issues/252 searchBar.addEventListener( 'focusin', () => { - // @ts-expect-error simFrame.contentWindow!.phet.joist.sim.display.simulationRoot.pdomVisible = false; } ); searchBar.addEventListener( 'focusout', () => { - // @ts-expect-error simFrame.contentWindow!.phet.joist.sim.display.simulationRoot.pdomVisible = true; } ); } @@ -292,7 +290,7 @@ let matchArray = nonEndingMatches; // separate storage for when the phetioID's component name matches the search - if ( phetio.PhetioIDUtils.getComponentName( treeNode.phetioID ).toLowerCase().includes( searchTerm.toLowerCase() ) ) { + if ( window.phetio.PhetioIDUtils.getComponentName( treeNode.phetioID ).toLowerCase().includes( searchTerm.toLowerCase() ) ) { matchArray = endingMatches; } matchArray.push( treeNode ); Index: models-of-the-hydrogen-atom/js/common/MOTHAQueryParameters.ts IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/models-of-the-hydrogen-atom/js/common/MOTHAQueryParameters.ts b/models-of-the-hydrogen-atom/js/common/MOTHAQueryParameters.ts --- a/models-of-the-hydrogen-atom/js/common/MOTHAQueryParameters.ts (revision 446cd03289dde9709ae9e4472f259c8f5021a881) +++ b/models-of-the-hydrogen-atom/js/common/MOTHAQueryParameters.ts (date 1693436707158) @@ -9,6 +9,7 @@ import logGlobal from '../../../phet-core/js/logGlobal.js'; import modelsOfTheHydrogenAtom from '../modelsOfTheHydrogenAtom.js'; +import { QueryStringMachineSchema } from '../../../chipper/phet-types.js'; const SCHEMA_MAP: Record = { Index: tandem/js/PhetioDynamicElementContainer.ts IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/tandem/js/PhetioDynamicElementContainer.ts b/tandem/js/PhetioDynamicElementContainer.ts --- a/tandem/js/PhetioDynamicElementContainer.ts (revision 10b19cd8579375a495507000287d041bd2c4395f) +++ b/tandem/js/PhetioDynamicElementContainer.ts (date 1693436518206) @@ -236,7 +236,7 @@ private stateSetOnAllChildrenOfDynamicElement( dynamicElementID: string, stillToSetIDs: string[] ): boolean { for ( let i = 0; i < stillToSetIDs.length; i++ ) { - if ( phetio.PhetioIDUtils.isAncestor( dynamicElementID, stillToSetIDs[ i ] ) ) { + if ( window.phetio.PhetioIDUtils.isAncestor( dynamicElementID, stillToSetIDs[ i ] ) ) { return false; } } Index: tandem/js/PhetioObject.ts IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/tandem/js/PhetioObject.ts b/tandem/js/PhetioObject.ts --- a/tandem/js/PhetioObject.ts (revision 10b19cd8579375a495507000287d041bd2c4395f) +++ b/tandem/js/PhetioObject.ts (date 1693436606286) @@ -746,7 +746,7 @@ const children = Object.keys( this.tandem.children ); const linkedChildren: LinkedElement[] = []; children.forEach( childName => { - const childPhetioID = phetio.PhetioIDUtils.append( this.tandem.phetioID, childName ); + const childPhetioID = window.phetio.PhetioIDUtils.append( this.tandem.phetioID, childName ); // Note that if it doesn't find a phetioID, that may be a synthetic node with children but not itself instrumented. const phetioObject: PhetioObject | undefined = phet.phetio.phetioEngine.phetioObjectMap[ childPhetioID ]; @@ -755,7 +755,7 @@ } } ); const linkedTandemNames = linkedChildren.map( ( linkedElement: LinkedElement ): string => { - return phetio.PhetioIDUtils.getComponentName( linkedElement.phetioID ); + return window.phetio.PhetioIDUtils.getComponentName( linkedElement.phetioID ); } ); let linkedChild: LinkedElement | null = null; if ( linkedChildren.length === 1 ) { Index: joist/js/QueryParametersWarningDialog.ts IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/joist/js/QueryParametersWarningDialog.ts b/joist/js/QueryParametersWarningDialog.ts --- a/joist/js/QueryParametersWarningDialog.ts (revision 3c8f2717688ca16889543fd2cc2766c59eec743a) +++ b/joist/js/QueryParametersWarningDialog.ts (date 1693436370327) @@ -13,6 +13,7 @@ import { Text } from '../../scenery/js/imports.js'; import joist from './joist.js'; import JoistStrings from './JoistStrings.js'; +import { Warning } from '../../chipper/phet-types.js'; type SelfOptions = EmptySelfOptions; export type QueryParametersWarningDialogOptions = SelfOptions & OopsDialogOptions; Index: scenery/js/nodes/RichText.ts IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/scenery/js/nodes/RichText.ts b/scenery/js/nodes/RichText.ts --- a/scenery/js/nodes/RichText.ts (revision 0f7341a2e6e257b640466fa418d28be4c180e7e8) +++ b/scenery/js/nodes/RichText.ts (date 1693436550296) @@ -109,7 +109,7 @@ export type RichTextLinks = RichTextLinksObject | true; // Used only for guarding against assertions, we want to know that we aren't in stringTesting mode -const isStringTest = window.QueryStringMachine && QueryStringMachine.containsKey( 'stringTest' ); +const isStringTest = window.hasOwnProperty( 'QueryStringMachine' ) && QueryStringMachine.containsKey( 'stringTest' ); type SelfOptions = { // Sets how bounds are determined for text Index: natural-selection/js/common/model/parseInitialPopulation.ts IDEA additional info: Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP <+>UTF-8 =================================================================== diff --git a/natural-selection/js/common/model/parseInitialPopulation.ts b/natural-selection/js/common/model/parseInitialPopulation.ts --- a/natural-selection/js/common/model/parseInitialPopulation.ts (revision 90feec8a1f6feefe52c608e8b69c2efde8331d4c) +++ b/natural-selection/js/common/model/parseInitialPopulation.ts (date 1693436750462) @@ -27,6 +27,7 @@ import BunnyVariety from './BunnyVariety.js'; import Gene from './Gene.js'; import GenePool from './GenePool.js'; +import IntentionalAny from '../../../../phet-core/js/types/IntentionalAny.js'; type AllelesPair = { fatherAllele: Allele | null; ```

[samreid]

This issue is getting out of hand, and it seems that in module mode, there is no way to declare a global so that it is available as both myGlobal and window.myGlobal. It would have to be repeated in global and global { interface Window}. It doesn't seem like the time to reorganize and clean all that up, and to decide on the preferable way to access globals (probably via window. though), and enforce it. Let's keep this issue about getting the types right for lodash and addressing the TS2686 that was originally reported. @zepumph confirmed the patch in https://github.com/phetsims/chipper/issues/1402#issuecomment-1699899889 worked as intended. So let's start with that.

[samreid]

I added the types for lodash, and I added documentation to explain the types files, and I cross referenced them. This issue seems good to close. There is a bunch of other technical debt in phet-types.d.ts but I'm not sure when it should be addressed. Closing.