Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve node vs browser tsconfig environments #1463

Closed
5 of 6 tasks
samreid opened this issue Sep 17, 2024 · 6 comments
Closed
5 of 6 tasks

Improve node vs browser tsconfig environments #1463

samreid opened this issue Sep 17, 2024 · 6 comments

Comments

@samreid
Copy link
Member

samreid commented Sep 17, 2024

In #1459, we added support for TypeScript in chipper via spawning processes. Next, we would like to improve the tsconfig environments for browser vs node:

  • Add some ts and tasks in aqua so we can use it as a test case here, overlaps somewhat with Convert grunt entry points to support Typescript #1464
  • Create separate tsconfig for browser vs node, with a shared base
  • Get node + browser working in the same repo. With different "files" or "excludes" etc.
  • Make sure lint works in both environments
  • Move browser-based/sim code to appropriate repos. LocalizedString etc should be in Joist. @zepumph is asking @jonathanolson about that. @jonathanolson confirmed OK to move things elsewhere.
  • Use nodenext module resolution
@samreid
Copy link
Member Author

samreid commented Sep 17, 2024

Some ideas for tsconfig, from discussion with @zepumph

Subject: [PATCH] Fix module resolution and import jimp, see https://github.com/phetsims/chipper/issues/1459
---
Index: js/getStringModule.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/js/getStringModule.ts b/js/getStringModule.ts
--- a/js/getStringModule.ts	(revision ce419be8db96fdfa2883db184e96eba02b056900)
+++ b/js/getStringModule.ts	(date 1726592587961)
@@ -41,6 +41,21 @@
   } );
 };
 
+process.env;
+process.blarg;
+
+document.hello;
+window.hello;
+global.hello;
+
+global.process;
+global.hello;
+
+window.location;
+
+const myDiv = document.createElement( 'div' );
+console.log( myDiv );
+
 const stringKeyToTandemName = ( key: string ) => {
   return key.replace( /(?:[-_\s]\w)/g, word => word[ 1 ].toUpperCase() );
 };
Index: tsconfig-node.json
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/tsconfig-node.json b/tsconfig-node.json
new file mode 100644
--- /dev/null	(date 1726592503627)
+++ b/tsconfig-node.json	(date 1726592503627)
@@ -0,0 +1,31 @@
+{
+  "extends": "./tsconfig-supercore.json",
+  "compilerOptions": {
+    // Treat files as modules even if it doesn't use import/export
+    "moduleDetection": "force",
+    // Ignore module structure
+    "module": "NodeNext",
+    // Allow JSON modules to be imported
+    "resolveJsonModule": true,
+    // Allow JS files to be imported from TS and vice versa
+    "allowJs": true,
+    // Use correct ESM import behavior
+    "esModuleInterop": true,
+    // Disallow features that require cross-file awareness
+    "isolatedModules": true,
+    "strict": true,
+    "moduleResolution": "NodeNext",
+    "target": "ESNext",
+    "lib": [
+      "ESNext"
+    ]
+  }
+}
+
+/*
+,
+"compilerOptions": {
+"esModuleInterop": true,
+"allowSyntheticDefaultImports": true,
+"moduleResolution": "node"
+}*/
Index: js/grunt/tasks/update.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/js/grunt/tasks/update.ts b/js/grunt/tasks/update.ts
--- a/js/grunt/tasks/update.ts	(revision ce419be8db96fdfa2883db184e96eba02b056900)
+++ b/js/grunt/tasks/update.ts	(date 1726592424359)
@@ -14,6 +14,7 @@
 import * as grunt from 'grunt';
 import getRepo from './util/getRepo';
 import * as fs from 'fs';
+import IntentionalAny from '../../../../phet-core/js/types/IntentionalAny.js';
 
 const repo = getRepo();
 
Index: tsconfig.json
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/tsconfig.json b/tsconfig.json
--- a/tsconfig.json	(revision ce419be8db96fdfa2883db184e96eba02b056900)
+++ b/tsconfig.json	(date 1726593261728)
@@ -6,9 +6,10 @@
     "mipmaps/**/*",
     "sounds/**/*"
   ],
-  "compilerOptions": {
-    "esModuleInterop": true,
-    "allowSyntheticDefaultImports": true,
-    "moduleResolution": "node"
-  }
+  "files": [
+    "js/Gruntfile.ts"
+  ],
+  "exclude": [
+    "js/grunt/**/*"
+  ]
 }
\ No newline at end of file
Index: js/grunt/tasks/clean.ts
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/js/grunt/tasks/clean.ts b/js/grunt/tasks/clean.ts
--- a/js/grunt/tasks/clean.ts	(revision ce419be8db96fdfa2883db184e96eba02b056900)
+++ b/js/grunt/tasks/clean.ts	(date 1726592587969)
@@ -6,8 +6,8 @@
  * @author Sam Reid (PhET Interactive Simulations)
  */
 
-import getRepo from './util/getRepo';
 import * as grunt from 'grunt';
+import getRepo from './util/getRepo';
 
 const repo = getRepo();
 const buildDirectory = `../${repo}/build`;
@@ -16,4 +16,19 @@
 if ( grunt.file.exists( buildDirectory ) ) {
   grunt.file.delete( buildDirectory );
 }
-grunt.file.mkdir( buildDirectory );
\ No newline at end of file
+grunt.file.mkdir( buildDirectory );
+
+process.env;
+process.blarg;
+
+document.hello;
+window.hello;
+global.hello;
+
+global.process;
+global.hello;
+
+window.location;
+
+const myDiv = document.createElement( 'div' );
+console.log( myDiv );
\ No newline at end of file
Index: tsconfig-supercore.json
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/tsconfig-supercore.json b/tsconfig-supercore.json
new file mode 100644
--- /dev/null	(date 1726592123623)
+++ b/tsconfig-supercore.json	(date 1726592123623)
@@ -0,0 +1,76 @@
+/**
+ * Shared defaults for tsconfig files.
+ * @author Sam Reid (PhET Interactive Simulations)
+ */
+{
+  "compilerOptions": {
+    /* Visit https://aka.ms/tsconfig.json to read more about this file */
+
+    /* Basic Options */
+    "incremental": true,                         /* Enable incremental compilation */
+    "target": "ES2020",                          /* Specify ECMAScript target version: 'ES3' (default), 'ES5', 'ES2015', 'ES2016', 'ES2017', 'ES2018', 'ES2019', 'ES2020', 'ES2021', or 'ESNEXT'. */
+    "module": "ES2020",                          /* Specify module code generation: 'none', 'commonjs', 'amd', 'system', 'umd', 'es2015', 'es2020', or 'ESNext'. */
+    // "lib": [],                                   /* Specify library files to be included in the compilation. */
+    "allowJs": true,                               /* Allow javascript files to be compiled. */
+    "checkJs": false,                              /* Report errors in .js files. */
+    "jsx": "react",                                /* Specify JSX code generation: 'preserve', 'react-native', 'react', 'react-jsx' or 'react-jsxdev'. */
+    "declaration": true,                            /* Generates corresponding '.d.ts' file. */
+    // "declarationMap": true,                      /* Generates a sourcemap for each corresponding '.d.ts' file. */
+    "sourceMap": false,                             /* Generates corresponding '.map' file. */
+    "outDir": "../chipper/dist/tsc/outDir",         /* Redirect output structure to the directory. */
+    "rootDir": "../",                               /* Specify the root directory of input files. Use to control the output directory structure with --outDir. */
+    // "rootDir": "./",                             /* Specify the root directory of input files. Use to control the output directory structure with --outDir. */
+    "composite": false,                             /* Enable project compilation (project references) */
+    // "tsBuildInfoFile": "./",                     /* Specify file to store incremental compilation information */
+    // "removeComments": true,                      /* Do not emit comments to output. */
+    "noEmit": false,                                /* Emit outputs. */
+    "emitDeclarationOnly": true,                    /* But not js output, because we use babel ourselves */
+    // "importHelpers": true,                       /* Import emit helpers from 'tslib'. */
+    // "downlevelIteration": true,                  /* Provide full support for iterables in 'for-of', spread, and destructuring when targeting 'ES5' or 'ES3'. */
+    "isolatedModules": true,                     /* Transpile each file as a separate module (similar to 'ts.transpileModule'). */
+
+    /* Strict Type-Checking Options */
+    "strict": true,                              /* Enable all strict type-checking options. */
+    // "noImplicitAny": true,                       /* Raise error on expressions and declarations with an implied 'any' type. */
+    // "strictNullChecks": true,                    /* Enable strict null checks. */
+    // "strictFunctionTypes": true,                 /* Enable strict checking of function types. */
+    // "strictBindCallApply": true,                 /* Enable strict 'bind', 'call', and 'apply' methods on functions. */
+    // "strictPropertyInitialization": true,        /* Enable strict checking of property initialization in classes. */
+    // "noImplicitThis": true,                      /* Raise error on 'this' expressions with an implied 'any' type. */
+    // "alwaysStrict": true,                        /* Parse in strict mode and emit "use strict" for each source file. */
+    /* Additional Checks */
+    // "noUnusedLocals": true,                      /* Report errors on unused locals. */
+    // "noUnusedParameters": true,                  /* Report errors on unused parameters. */
+    "noImplicitReturns": true,                      /* Report error when not all code paths in function return a value. */
+    // "noFallthroughCasesInSwitch": true,          /* Report errors for fallthrough cases in switch statement. */
+    // "noUncheckedIndexedAccess": true,            /* Include 'undefined' in index signature results */
+    "noImplicitOverride": true,                     /* Ensure overriding members in derived classes are marked with an 'override' modifier. */
+    // "noPropertyAccessFromIndexSignature": true,  /* Require undeclared properties from index signatures to use element accesses. */
+
+    /* Module Resolution Options */
+    // "moduleResolution": "node",                  /* Specify module resolution strategy: 'node' (Node.js) or 'classic' (TypeScript pre-1.6). */
+    // "baseUrl": "./",                             /* Base directory to resolve non-absolute module names. */
+    // "paths": {},                                 /* A series of entries which re-map imports to lookup locations relative to the 'baseUrl'. */
+    // "rootDirs": [],                              /* List of root folders whose combined content represents the structure of the project at runtime. */
+    // "typeRoots": [],                             /* List of folders to include type definitions from. */
+    // "types": [],                                 /* Type declaration files to be included in compilation. */
+    // "allowSyntheticDefaultImports": true,        /* Allow default imports from modules with no default export. This does not affect code emit, just typechecking. */
+    // "esModuleInterop": true,                     /* Enables emit interoperability between CommonJS and ES Modules via creation of namespace objects for all imports. Implies 'allowSyntheticDefaultImports'. */
+    // "preserveSymlinks": true,                    /* Do not resolve the real path of symlinks. */
+    "allowUmdGlobalAccess": true,                /* Allow accessing UMD globals from modules. */
+
+    /* Source Map Options */
+    // "sourceRoot": "",                            /* Specify the location where debugger should locate TypeScript files instead of source locations. */
+    // "mapRoot": "",                               /* Specify the location where debugger should locate map files instead of generated locations. */
+    // "inlineSourceMap": true,                     /* Emit a single file with source maps instead of having a separate file. */
+    // "inlineSources": true,                       /* Emit the source alongside the sourcemaps within a single file; requires '--inlineSourceMap' or '--sourceMap' to be set. */
+
+    /* Experimental Options */
+    // "experimentalDecorators": true,              /* Enables experimental support for ES7 decorators. */
+    // "emitDecoratorMetadata": true,               /* Enables experimental support for emitting type metadata for decorators. */
+
+    /* Advanced Options */
+    "skipLibCheck": true,                           /* Skip type checking of declaration files. */
+    "forceConsistentCasingInFileNames": true        /* Disallow inconsistently-cased references to the same file. */
+  }
+}
\ No newline at end of file
Index: tsconfig-core.json
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/tsconfig-core.json b/tsconfig-core.json
--- a/tsconfig-core.json	(revision ce419be8db96fdfa2883db184e96eba02b056900)
+++ b/tsconfig-core.json	(date 1726592666014)
@@ -3,75 +3,11 @@
  * @author Sam Reid (PhET Interactive Simulations)
  */
 {
+  "extends": "./tsconfig-supercore.json",
   "compilerOptions": {
-    /* Visit https://aka.ms/tsconfig.json to read more about this file */
-
-    /* Basic Options */
-    "incremental": true,                         /* Enable incremental compilation */
-    "target": "ES2020",                          /* Specify ECMAScript target version: 'ES3' (default), 'ES5', 'ES2015', 'ES2016', 'ES2017', 'ES2018', 'ES2019', 'ES2020', 'ES2021', or 'ESNEXT'. */
-    "module": "ES2020",                          /* Specify module code generation: 'none', 'commonjs', 'amd', 'system', 'umd', 'es2015', 'es2020', or 'ESNext'. */
-    // "lib": [],                                   /* Specify library files to be included in the compilation. */
-    "allowJs": true,                               /* Allow javascript files to be compiled. */
-    "checkJs": false,                              /* Report errors in .js files. */
-    "jsx": "react",                                /* Specify JSX code generation: 'preserve', 'react-native', 'react', 'react-jsx' or 'react-jsxdev'. */
-    "declaration": true,                            /* Generates corresponding '.d.ts' file. */
-    // "declarationMap": true,                      /* Generates a sourcemap for each corresponding '.d.ts' file. */
-    "sourceMap": false,                             /* Generates corresponding '.map' file. */
-    "outDir": "../chipper/dist/tsc/outDir",         /* Redirect output structure to the directory. */
-    "rootDir": "../",                               /* Specify the root directory of input files. Use to control the output directory structure with --outDir. */
-    // "rootDir": "./",                             /* Specify the root directory of input files. Use to control the output directory structure with --outDir. */
-    "composite": false,                             /* Enable project compilation (project references) */
-    // "tsBuildInfoFile": "./",                     /* Specify file to store incremental compilation information */
-    // "removeComments": true,                      /* Do not emit comments to output. */
-    "noEmit": false,                                /* Emit outputs. */
-    "emitDeclarationOnly": true,                    /* But not js output, because we use babel ourselves */
-    // "importHelpers": true,                       /* Import emit helpers from 'tslib'. */
-    // "downlevelIteration": true,                  /* Provide full support for iterables in 'for-of', spread, and destructuring when targeting 'ES5' or 'ES3'. */
-     "isolatedModules": true,                     /* Transpile each file as a separate module (similar to 'ts.transpileModule'). */
-
-    /* Strict Type-Checking Options */
-    "strict": true,                              /* Enable all strict type-checking options. */
-    // "noImplicitAny": true,                       /* Raise error on expressions and declarations with an implied 'any' type. */
-    // "strictNullChecks": true,                    /* Enable strict null checks. */
-    // "strictFunctionTypes": true,                 /* Enable strict checking of function types. */
-    // "strictBindCallApply": true,                 /* Enable strict 'bind', 'call', and 'apply' methods on functions. */
-    // "strictPropertyInitialization": true,        /* Enable strict checking of property initialization in classes. */
-    // "noImplicitThis": true,                      /* Raise error on 'this' expressions with an implied 'any' type. */
-    // "alwaysStrict": true,                        /* Parse in strict mode and emit "use strict" for each source file. */
-    /* Additional Checks */
-    // "noUnusedLocals": true,                      /* Report errors on unused locals. */
-    // "noUnusedParameters": true,                  /* Report errors on unused parameters. */
-    "noImplicitReturns": true,                      /* Report error when not all code paths in function return a value. */
-    // "noFallthroughCasesInSwitch": true,          /* Report errors for fallthrough cases in switch statement. */
-    // "noUncheckedIndexedAccess": true,            /* Include 'undefined' in index signature results */
-    "noImplicitOverride": true,                     /* Ensure overriding members in derived classes are marked with an 'override' modifier. */
-    // "noPropertyAccessFromIndexSignature": true,  /* Require undeclared properties from index signatures to use element accesses. */
-
-    /* Module Resolution Options */
-    // "moduleResolution": "node",                  /* Specify module resolution strategy: 'node' (Node.js) or 'classic' (TypeScript pre-1.6). */
-    // "baseUrl": "./",                             /* Base directory to resolve non-absolute module names. */
-    // "paths": {},                                 /* A series of entries which re-map imports to lookup locations relative to the 'baseUrl'. */
-    // "rootDirs": [],                              /* List of root folders whose combined content represents the structure of the project at runtime. */
-    // "typeRoots": [],                             /* List of folders to include type definitions from. */
-    // "types": [],                                 /* Type declaration files to be included in compilation. */
-    // "allowSyntheticDefaultImports": true,        /* Allow default imports from modules with no default export. This does not affect code emit, just typechecking. */
-    // "esModuleInterop": true,                     /* Enables emit interoperability between CommonJS and ES Modules via creation of namespace objects for all imports. Implies 'allowSyntheticDefaultImports'. */
-    // "preserveSymlinks": true,                    /* Do not resolve the real path of symlinks. */
-     "allowUmdGlobalAccess": true,                /* Allow accessing UMD globals from modules. */
-
-    /* Source Map Options */
-    // "sourceRoot": "",                            /* Specify the location where debugger should locate TypeScript files instead of source locations. */
-    // "mapRoot": "",                               /* Specify the location where debugger should locate map files instead of generated locations. */
-    // "inlineSourceMap": true,                     /* Emit a single file with source maps instead of having a separate file. */
-    // "inlineSources": true,                       /* Emit the source alongside the sourcemaps within a single file; requires '--inlineSourceMap' or '--sourceMap' to be set. */
-
-    /* Experimental Options */
-    // "experimentalDecorators": true,              /* Enables experimental support for ES7 decorators. */
-    // "emitDecoratorMetadata": true,               /* Enables experimental support for emitting type metadata for decorators. */
-
-    /* Advanced Options */
-    "skipLibCheck": true,                           /* Skip type checking of declaration files. */
-    "forceConsistentCasingInFileNames": true        /* Disallow inconsistently-cased references to the same file. */
+    "lib": [
+      "DOM"
+    ]
   },
   "files": [
     "phet-types.d.ts",
Index: js/grunt/tsconfig.json
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/js/grunt/tsconfig.json b/js/grunt/tsconfig.json
new file mode 100644
--- /dev/null	(date 1726591845136)
+++ b/js/grunt/tsconfig.json	(date 1726591845136)
@@ -0,0 +1,6 @@
+{
+  "extends": "../../tsconfig-node.json",
+  "include": [
+    "**/*"
+  ]
+}
\ No newline at end of file

@zepumph
Copy link
Member

zepumph commented Sep 27, 2024

We got to a commit point in aqua, and have a working solution. It relies on needing "top down" and "bottom up" tsconfig detection, depending on if you run tsc in the root of the repo, or if you want webstorm/eslint to check one file, needing to search up the hierarchy. Hopefully one day those will be more similar, but for now aqua is our exemplar.

  • It is best to support browser and node as two complete and separate projects within a repo. Don’t specify two different projects that are both node, and need to depend on each other, instead have them both extend a single tsconfig that “include”s all node directories
  • Files and include always override and never augment. If neither is specified, typescript defaults to taking everything.
  • ###Resolving Type Information
  • For WebStorm, and for eslint with projectService: true:
    the types for a given *.ts file, it follows a resolution strategy that searches for the nearest directory or parent directory with a tsconfig.json file.
  • For command line tsc: we want to top level tsconfig to be the central location for type checking, so it should include both project references (browser and node). This doesn’t support webstorm/eslint (see above). So we need a “top down” central point per repo, as well as a “bottom up” approach for webstorm and eslint.

samreid added a commit to phetsims/aqua that referenced this issue Sep 28, 2024
zepumph added a commit that referenced this issue Sep 30, 2024
zepumph added a commit that referenced this issue Sep 30, 2024
zepumph added a commit to phetsims/aqua that referenced this issue Sep 30, 2024
zepumph added a commit to phetsims/aqua that referenced this issue Sep 30, 2024
zepumph added a commit to phetsims/perennial that referenced this issue Sep 30, 2024
zepumph added a commit to phetsims/perennial that referenced this issue Sep 30, 2024
zepumph added a commit to phetsims/perennial that referenced this issue Sep 30, 2024
zepumph added a commit that referenced this issue Sep 30, 2024
zepumph added a commit to phetsims/perennial that referenced this issue Oct 4, 2024
@zepumph
Copy link
Member

zepumph commented Oct 4, 2024

  • Do we want to force outside sim developers to type check our build tools in order to build their sim? I think so, but perhaps it is worth a conversation.

@zepumph
Copy link
Member

zepumph commented Oct 4, 2024

Primary starting point for current work:

  1. development on: kite,dot,aqua,chipper,perennial(-alias).
  2. npm prune && npm update in chipper, perennial, perennial-alias
  3. Lint everything and tsc all will likely show problems, and can get us on track for getting chipper's linting to respect node vs browser.

zepumph added a commit to phetsims/perennial that referenced this issue Oct 4, 2024
zepumph added a commit that referenced this issue Oct 4, 2024
zepumph added a commit that referenced this issue Oct 4, 2024
samreid pushed a commit to phetsims/perennial that referenced this issue Oct 21, 2024
samreid pushed a commit to phetsims/perennial that referenced this issue Oct 21, 2024
samreid pushed a commit to phetsims/perennial that referenced this issue Oct 21, 2024
samreid pushed a commit to phetsims/perennial that referenced this issue Oct 21, 2024
samreid pushed a commit to phetsims/perennial that referenced this issue Oct 21, 2024
samreid pushed a commit to phetsims/perennial that referenced this issue Oct 21, 2024
samreid added a commit to phetsims/perennial that referenced this issue Oct 21, 2024
samreid added a commit to phetsims/perennial that referenced this issue Oct 21, 2024
samreid added a commit to phetsims/perennial that referenced this issue Oct 21, 2024
samreid added a commit to phetsims/perennial that referenced this issue Oct 21, 2024
samreid added a commit to phetsims/perennial that referenced this issue Oct 21, 2024
samreid pushed a commit to phetsims/perennial that referenced this issue Oct 21, 2024
samreid pushed a commit to phetsims/perennial that referenced this issue Oct 22, 2024
samreid added a commit to phetsims/perennial that referenced this issue Oct 22, 2024
zepumph added a commit to phetsims/perennial that referenced this issue Oct 22, 2024
zepumph added a commit to phetsims/perennial that referenced this issue Oct 22, 2024
zepumph pushed a commit to phetsims/perennial that referenced this issue Oct 22, 2024
samreid pushed a commit that referenced this issue Oct 23, 2024
samreid added a commit that referenced this issue Oct 23, 2024
samreid pushed a commit that referenced this issue Nov 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants