ref: Enable noUncheckedIndexedAccess TS config (#12461)

This PR enables the `noUncheckedIndexedAccess` TS config, ensuring that
we always properly guard for array/object access.

Sometimes, this leads to a bit of boilerplate, but enabling this def.
found some places in the code where things could go wrong before.

Closes #12440

Afterwards, we can look into enabling
https://typescript-eslint.io/rules/no-unnecessary-condition/.

Author: mydea

.github/workflows/build.yml

@@ -896,15 +896,15 @@ jobs:
 
       - name: Overwrite typescript version
         if: matrix.typescript
-        run: yarn add --dev --ignore-workspace-root-check typescript@${{ matrix.typescript }}
+        run: node ./scripts/use-ts-version.js ${{ matrix.typescript }}
+        working-directory: dev-packages/node-integration-tests
 
       - name: Run integration tests
         env:
           NODE_VERSION: ${{ matrix.node }}
           TS_VERSION: ${{ matrix.typescript }}
-        run: |
-          cd dev-packages/node-integration-tests
-          yarn test
+        working-directory: dev-packages/node-integration-tests
+        run: yarn test
 
   job_remix_integration_tests:
     name: Remix v${{ matrix.remix }} (Node ${{ matrix.node }}) Tests

dev-packages/node-integration-tests/scripts/use-ts-version.js

@@ -0,0 +1,23 @@
+/* eslint-disable no-console */
+const { execSync } = require('child_process');
+const { join } = require('path');
+const { writeFileSync } = require('fs');
+
+const cwd = join(__dirname, '../../..');
+
+const tsVersion = process.argv[2] || '3.8';
+
+console.log(`Installing typescript@${tsVersion}...`);
+
+execSync(`yarn add --dev --ignore-workspace-root-check typescript@${tsVersion}`, { stdio: 'inherit', cwd });
+
+console.log('Removing unsupported tsconfig options...');
+
+const baseTscConfigPath = join(cwd, 'packages/typescript/tsconfig.json');
+
+const tsConfig = require(baseTscConfigPath);
+
+// TS 3.8 fails build when it encounteres a config option it does not understand, so we remove it :(
+delete tsConfig.compilerOptions.noUncheckedIndexedAccess;
+
+writeFileSync(baseTscConfigPath, JSON.stringify(tsConfig, null, 2));

dev-packages/node-integration-tests/suites/express/multiple-routers/complex-router/test.ts

@@ -9,7 +9,7 @@ afterAll(() => {
 conditionalTest({ min: 16 })('complex-router', () => {
   test('should construct correct url with multiple parameterized routers, when param is also contain in middle layer route and express used multiple middlewares with route', done => {
     // parse node.js major version
-    const [major] = process.versions.node.split('.').map(Number);
+    const [major = 0] = process.versions.node.split('.').map(Number);
     // Split test result base on major node version because regex d flag is support from node 16+
 
     const EXPECTED_TRANSACTION =
@@ -36,7 +36,7 @@ conditionalTest({ min: 16 })('complex-router', () => {
 
   test('should construct correct url with multiple parameterized routers, when param is also contain in middle layer route and express used multiple middlewares with route and original url has query params', done => {
     // parse node.js major version
-    const [major] = process.versions.node.split('.').map(Number);
+    const [major = 0] = process.versions.node.split('.').map(Number);
     // Split test result base on major node version because regex d flag is support from node 16+
     const EXPECTED_TRANSACTION =
       major >= 16
@@ -62,7 +62,7 @@ conditionalTest({ min: 16 })('complex-router', () => {
 
   test('should construct correct url with multiple parameterized routers, when param is also contain in middle layer route and express used multiple middlewares with route and original url ends with trailing slash and has query params', done => {
     // parse node.js major version
-    const [major] = process.versions.node.split('.').map(Number);
+    const [major = 0] = process.versions.node.split('.').map(Number);
     // Split test result base on major node version because regex d flag is support from node 16+
     const EXPECTED_TRANSACTION =
       major >= 16

dev-packages/node-integration-tests/suites/express/multiple-routers/middle-layer-parameterized/test.ts

@@ -9,7 +9,7 @@ afterAll(() => {
 conditionalTest({ min: 16 })('middle-layer-parameterized', () => {
   test('should construct correct url with multiple parameterized routers, when param is also contain in middle layer route', done => {
     // parse node.js major version
-    const [major] = process.versions.node.split('.').map(Number);
+    const [major = 0] = process.versions.node.split('.').map(Number);
     // Split test result base on major node version because regex d flag is support from node 16+
     const EXPECTED_TRANSACTION =
       major >= 16

dev-packages/node-integration-tests/suites/public-api/LocalVariables/test.ts

@@ -44,7 +44,7 @@ conditionalTest({ min: 18 })('LocalVariables integration', () => {
       .ignore('session')
       .expect({
         event: event => {
-          for (const frame of event.exception?.values?.[0].stacktrace?.frames || []) {
+          for (const frame of event.exception?.values?.[0]?.stacktrace?.frames || []) {
             expect(frame.vars).toBeUndefined();
           }
         },

dev-packages/node-integration-tests/suites/public-api/startSpan/with-nested-spans/test.ts

@@ -10,7 +10,7 @@ test('should report finished spans as children of the root transaction.', done =
     .expect({
       transaction: transaction => {
         const rootSpanId = transaction.contexts?.trace?.span_id;
-        const span3Id = transaction.spans?.[1].span_id;
+        const span3Id = transaction.spans?.[1]?.span_id;
 
         expect(rootSpanId).toEqual(expect.any(String));
         expect(span3Id).toEqual(expect.any(String));

dev-packages/node-integration-tests/suites/tsconfig.json

@@ -0,0 +1,3 @@
+{
+  "extends": "../tsconfig.test.json",
+}

dev-packages/node-integration-tests/utils/index.ts

@@ -196,7 +196,13 @@ export class TestEnv {
    * @returns The extracted envelope.
    */
   public async getEnvelopeRequest(options?: DataCollectorOptions): Promise<Array<Record<string, unknown>>> {
-    return (await this.getMultipleEnvelopeRequest({ ...options, count: 1 }))[0];
+    const requests = await this.getMultipleEnvelopeRequest({ ...options, count: 1 });
+
+    if (!requests[0]) {
+      throw new Error('No requests found');
+    }
+
+    return requests[0];
   }
 
   /**
@@ -239,7 +245,7 @@ export class TestEnv {
         .post('/api/1337/envelope/', body => {
           const envelope = parseEnvelope(body);
 
-          if (envelopeType.includes(envelope[1].type as EnvelopeItemType)) {
+          if (envelopeType.includes(envelope[1]?.type as EnvelopeItemType)) {
             envelopes.push(envelope);
           } else {
             return false;
@@ -296,7 +302,7 @@ export class TestEnv {
         .post('/api/1337/envelope/', body => {
           const envelope = parseEnvelope(body);
 
-          if (options.envelopeType.includes(envelope[1].type as EnvelopeItemType)) {
+          if (options.envelopeType.includes(envelope[1]?.type as EnvelopeItemType)) {
             reqCount++;
             return true;
           }

dev-packages/overhead-metrics/src/util/github.ts

@@ -13,7 +13,7 @@ const octokit = new Octokit({
   // log: console,
 });
 
-const [, owner, repo] = (await Git.repository).split('/');
+const [, owner, repo] = (await Git.repository).split('/') as [string, string, string];
 const defaultArgs = { owner, repo };
 
 async function downloadArtifact(url: string, path: string): Promise<void> {
@@ -51,15 +51,10 @@ async function tryAddOrUpdateComment(commentBuilder: PrCommentBuilder): Promise<
     For example, refs/heads/feature-branch-1.
   */
   let prNumber: number | undefined;
-  if (
-    typeof process.env.GITHUB_REF == 'string' &&
-    process.env.GITHUB_REF.length > 0 &&
-    process.env.GITHUB_REF.startsWith('refs/pull/')
-  ) {
-    prNumber = parseInt(process.env.GITHUB_REF.split('/')[2]);
-    console.log(
-      `Determined PR number ${prNumber} based on GITHUB_REF environment variable: '${process.env.GITHUB_REF}'`,
-    );
+  const githubRef = process.env.GITHUB_REF;
+  if (typeof githubRef == 'string' && githubRef.length > 0 && githubRef.startsWith('refs/pull/')) {
+    prNumber = parseInt(githubRef.split('/')[2] as string);
+    console.log(`Determined PR number ${prNumber} based on GITHUB_REF environment variable: '${githubRef}'`);
   } else if (!(await Git.branchIsBase)) {
     prNumber = (
       await octokit.rest.pulls.list({
@@ -155,15 +150,17 @@ export const GitHub = {
         status: 'success',
       });
 
-      if (workflowRuns.data.total_count == 0) {
+      const firstRun = workflowRuns.data.workflow_runs[0];
+
+      if (workflowRuns.data.total_count == 0 || !firstRun) {
         console.warn(`Couldn't find any successful run for workflow '${workflow.name}'`);
         return;
       }
 
       const artifact = (
         await octokit.actions.listWorkflowRunArtifacts({
           ...defaultArgs,
-          run_id: workflowRuns.data.workflow_runs[0].id,
+          run_id: firstRun.id,
         })
       ).data.artifacts.find(it => it.name == artifactName);
 

dev-packages/test-utils/src/event-proxy-server.ts

@@ -59,7 +59,7 @@ export async function startEventProxyServer(options: EventProxyServerOptions): P
           ? zlib.gunzipSync(Buffer.concat(proxyRequestChunks)).toString()
           : Buffer.concat(proxyRequestChunks).toString();
 
-      const envelopeHeader: EnvelopeItem[0] = JSON.parse(proxyRequestBody.split('\n')[0]);
+      const envelopeHeader: EnvelopeItem[0] = JSON.parse(proxyRequestBody.split('\n')[0] as string);
 
       const shouldForwardEventToSentry = options.forwardToSentry != null ? options.forwardToSentry : true;