Skip to content

Bump eslint/typescript and update Typescript - #450

Open
JustinGrote wants to merge 3 commits into
github:mainfrom
JustinGrote:chore/ts-eslint-bump
Open

Bump eslint/typescript and update Typescript#450
JustinGrote wants to merge 3 commits into
github:mainfrom
JustinGrote:chore/ts-eslint-bump

Conversation

@JustinGrote

Copy link
Copy Markdown

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.

CopilotAI review requested due to automatic review settings March 8, 2025 19:38
@JustinGrote
JustinGrote requested a review from a team as a code ownerMarch 8, 2025 19:38

CopilotAI left a comment

Copy link
Copy Markdown

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

FileDescription
eslint.config.mjsAdds 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
Copy Markdown
Author

@AnthonyBorton please review, thanks :)

@joshmgross

Copy link
Copy Markdown
Contributor

@JustinGrote

Copy link
Copy Markdown
Author

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

CopilotAI left a comment

Copy link
Copy Markdown

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
FileDescription
eslint.config.mjsNew mjs configuration file integrating ESLint and typescript-eslint recommended configs.
.github/workflows/build.ymlUpdated Node version to 20.x for the build process.
src/treeViews/icons.tsUpdated icon helper functions; changed return type of getIconForWorkflowStep.
src/treeViews/shared/workflowJobNode.tsRefactored constructor formatting for clarity.
src/treeViews/shared/workflowRunNode.tsRemoved 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

JustinGrote commented Mar 11, 2025

Copy link
Copy Markdown
Author

@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
JustinGroteforce-pushed the chore/ts-eslint-bump branch 2 times, most recently from b986529 to 805146bCompareMarch 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

CopilotAI left a comment

Copy link
Copy Markdown

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
FileDescription
eslint.config.mjsAdded new mjs ESLint configuration with TypeScript and Prettier settings.
.github/workflows/build.ymlUpdated Node.js version from 16.x to 20.x in the build workflow.
src/pinnedWorkflows/pinnedWorkflows.tsAdded TS suppression comments for listing repo workflows and mapping workflow paths.
src/store/workflowRun.tsAdded TS suppression comments for listing jobs for workflow runs and attempts.
src/treeViews/settings/environmentSecretsNode.tsAdded TS suppression comments for listing environment secrets.
src/treeViews/settings/repoSecretsNode.tsAdded TS suppression comments for listing repository secrets.
src/treeViews/icons.tsUpdated icon helper function return types and signatures for workflow run and step icons.
src/treeViews/settings/environmentVariablesNode.tsAdded TS suppression comments for listing environment variables.
src/commands/openWorkflowFile.tsSimplified error handling in the open workflow file command.
src/treeViews/settings/repoVariablesNode.tsAdded TS suppression comments for listing repository variables.
src/treeViews/shared/workflowJobNode.tsRefactored constructor formatting for better readability.
src/treeViews/shared/previousAttemptsNode.tsRefactored constructor formatting for consistency.
src/treeViews/shared/attemptNode.tsRefactored constructor formatting to multi-line parameters.
src/treeViews/current-branch/currentBranchRepoNode.tsRefactored constructor formatting for clarity.
src/treeViews/settings/environmentNode.tsRefactored 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
Copy Markdown
Author

@joshmgross friendly bump :)

@joshmgrossjoshmgross self-assigned this Mar 18, 2025
@joshmgrossjoshmgross removed their assignment Jun 16, 2025
@JustinGrote

Copy link
Copy Markdown
Author

@joshmgross you removed yourself from this PR. Who should I be reviewing this with?

@JustinGrote
JustinGroteforce-pushed the chore/ts-eslint-bump branch 2 times, most recently from 48cd3ee to b2c15d8CompareJuly 2, 2025 00:03
Sign up for freeto 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.

3 participants

@JustinGrote@joshmgross