Skip to content

Bump eslint/typescript and update Typescript #450

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

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

JustinGrote
Copy link

Update Typescript, eslint, and typescript-eslint to latest version and switch to mjs style configuration.

This PR is a dependency for my other PRs to avoid linting errors.

@Copilot Copilot AI review requested due to automatic review settings March 8, 2025 19:38
@JustinGrote JustinGrote requested a review from a team as a code owner March 8, 2025 19:38
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PR Overview

This PR upgrades Typescript, eslint, and typescript-eslint to the latest versions and transitions to an mjs style configuration to address linting errors.

  • Introduces a new mjs configuration file for eslint.
  • Ensures the configuration aggregates recommended settings from eslint and typescript-eslint.

Reviewed Changes

File Description
eslint.config.mjs Adds a new mjs configuration file combining recommended settings from eslint and typescript-eslint.

Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.

@JustinGrote
Copy link
Author

@AnthonyBorton please review, thanks :)

@joshmgross
Copy link
Member

@JustinGrote thanks for this!

Could you look into the build failures?
https://github.com/github/vscode-github-actions/actions/runs/13740852110/job/38577940818?pr=450

@JustinGrote
Copy link
Author

Newer eslint has diff options, I'll update the pipeline

@JustinGrote JustinGrote requested a review from Copilot March 11, 2025 23:31
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR updates dependency versions and configuration for Typescript, ESLint, and typescript-eslint while switching to an mjs style configuration to address linting errors in upcoming PRs.

  • Added a new ESLint configuration file (eslint.config.mjs) that integrates recommended settings from ESLint and typescript-eslint.
  • Updated the build workflow to use Node version 20.x.
  • Adjusted icon utility functions including changes to return types in icons.ts and minor refactorings in tree view nodes.

Reviewed Changes

Copilot reviewed 13 out of 13 changed files in this pull request and generated no comments.

Show a summary per file
File Description
eslint.config.mjs New mjs configuration file integrating ESLint and typescript-eslint recommended configs.
.github/workflows/build.yml Updated Node version to 20.x for the build process.
src/treeViews/icons.ts Updated icon helper functions; changed return type of getIconForWorkflowStep.
src/treeViews/shared/workflowJobNode.ts Refactored constructor formatting for clarity.
src/treeViews/shared/workflowRunNode.ts Removed an extraneous newline to improve code clarity.
Comments suppressed due to low confidence (1)

src/treeViews/icons.ts:87

  • The change from returning an empty string to undefined in getIconForWorkflowStep may impact downstream consumers expecting a string. Verify that all usages properly handle an undefined value.
return undefined;

@JustinGrote
Copy link
Author

JustinGrote commented Mar 11, 2025

@joshmgross fixed up, please re-enable the workflow to test.

It passed in my Actions: https://github.com/JustinGrote/vscode-github-actions-enhanced/actions/runs/13800804697/job/38602688747

b2c15d8 is a summary commit of the changes since last review

@JustinGrote JustinGrote force-pushed the chore/ts-eslint-bump branch 2 times, most recently from b986529 to 805146b Compare March 12, 2025 00:15
- Bump latest esbuild version explicity
- Set CI Node to VSCode 1.98.0 Node Version
- Fixup vscode tooling and enable format-on-save
- Apply prettier configuration for whitespace remediation
- Suppress new typescript warnings from newer TS version, to be fixed later
@JustinGrote JustinGrote force-pushed the chore/ts-eslint-bump branch from 805146b to b2c15d8 Compare March 12, 2025 00:18
@JustinGrote JustinGrote requested a review from Copilot March 12, 2025 00:21
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR updates dependency versions for TypeScript, ESLint, and typescript-eslint and migrates to an mjs configuration style to resolve linting errors. The changes include upgrading the Node.js version in the CI workflow, adding TS-expect-error annotations where newer TypeScript versions report issues, and refactoring some constructor signatures and icon helper functions.

Reviewed Changes

Copilot reviewed 25 out of 25 changed files in this pull request and generated no comments.

Show a summary per file
File Description
eslint.config.mjs Added new mjs ESLint configuration with TypeScript and Prettier settings.
.github/workflows/build.yml Updated Node.js version from 16.x to 20.x in the build workflow.
src/pinnedWorkflows/pinnedWorkflows.ts Added TS suppression comments for listing repo workflows and mapping workflow paths.
src/store/workflowRun.ts Added TS suppression comments for listing jobs for workflow runs and attempts.
src/treeViews/settings/environmentSecretsNode.ts Added TS suppression comments for listing environment secrets.
src/treeViews/settings/repoSecretsNode.ts Added TS suppression comments for listing repository secrets.
src/treeViews/icons.ts Updated icon helper function return types and signatures for workflow run and step icons.
src/treeViews/settings/environmentVariablesNode.ts Added TS suppression comments for listing environment variables.
src/commands/openWorkflowFile.ts Simplified error handling in the open workflow file command.
src/treeViews/settings/repoVariablesNode.ts Added TS suppression comments for listing repository variables.
src/treeViews/shared/workflowJobNode.ts Refactored constructor formatting for better readability.
src/treeViews/shared/previousAttemptsNode.ts Refactored constructor formatting for consistency.
src/treeViews/shared/attemptNode.ts Refactored constructor formatting to multi-line parameters.
src/treeViews/current-branch/currentBranchRepoNode.ts Refactored constructor formatting for clarity.
src/treeViews/settings/environmentNode.ts Refactored constructor formatting for uniformity.
Comments suppressed due to low confidence (2)

src/pinnedWorkflows/pinnedWorkflows.ts:109

  • [nitpick] Consider adding a reference to a tracked issue or more detailed context in the ts-expect-error comment to help maintainers know when this temporary suppression can be removed.
      // @ts-expect-error FIXME: Newer Typescript catches a problem that previous didn't. This will be fixed in Octokit bump.

src/treeViews/icons.ts:87

  • Changing the return value from an empty string to undefined in getIconForWorkflowStep might lead to unexpected behavior if downstream code expects a string; ensure that all consumers can handle an undefined value.
  return undefined;

@JustinGrote
Copy link
Author

@joshmgross friendly bump :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants