Skip to content

chore: disallow using any as type#43

Merged
nicomiguelino merged 9 commits intoScreenly:masterfrom
nicomiguelino:disallow-any
Mar 26, 2025
Merged

chore: disallow using any as type#43
nicomiguelino merged 9 commits intoScreenly:masterfrom
nicomiguelino:disallow-any

Conversation

@nicomiguelino
Copy link
Copy Markdown
Collaborator

@nicomiguelino nicomiguelino commented Mar 25, 2025

Description

Removes the usage of any as a TypeScript type

How Has This Been Tested?

  • Unit tests
  • Manual tests

Checklist

  • I have performed a self-review of my code.
  • I have commented my code, particularly in hard-to-understand areas.
  • I have made corresponding changes to the documentation (where applicable).
  • New and existing unit tests pass locally with my changes
  • Any dependent changes have been merged and published in downstream modules

@nicomiguelino nicomiguelino requested a review from Copilot March 26, 2025 04:36
Copy link
Copy Markdown

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 disallows the usage of the TypeScript any type and enforces strict type checking throughout the codebase. Key changes include:

  • Updating ESLint configuration to use TypeScript-specific rules and disable the base no-unused-vars rule.
  • Refactoring utilities and action functions to use proper TypeScript types (replacing any with explicit types).
  • Adjusting tests to expect response data using data instead of json.

Reviewed Changes

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

Show a summary per file
File Description
.eslintrc.cjs Updated ESLint config to enforce strict TypeScript checks
src/types/screenly.ts Added TypeScript interfaces for Screenly API responses
test/helper-functions.test.ts Updated tests for helper functions to align with response.data usage
test/utils.test.ts Refactored tests to expect data instead of json in the response object
src/utils.ts Refactored utility functions to use proper types and remove unused makeRequest
src/actions/schedule-playlist-item.ts Added explicit type annotations and API key guard
src/actions/complete-workflow.ts Updated type annotations and switched variable types; some response handling may require updating
src/actions/cleanup-zapier-content.ts Updated type annotations and streamlined mapping of playlist IDs
src/triggers/* Updated triggers to use ZObject and Bundle for proper type checking
src/actions/upload-asset.ts Added explicit type annotations for action functions
src/triggers/get-playlists.ts Updated trigger to use proper types
src/triggers/get-screens.ts Updated trigger to use proper types
src/actions/assign-screen-to-playlist.ts Added explicit type annotations to action functions
Files not reviewed (1)
  • package.json: Language not supported
Comments suppressed due to low confidence (2)

src/actions/complete-workflow.ts:93

  • The code still references 'json' for response data, but the updated utils.handleError returns the 'data' property. Please update this line to use 'labelQueryResponse.data' to prevent runtime errors.
const existingLabels = labelQueryResponse.json;

src/utils.ts:6

  • The makeRequest function has been removed. Verify that no other parts of the code depend on makeRequest and, if they do, refactor them to use the updated request handling with proper error handling.
/* Removed makeRequest function */

@nicomiguelino nicomiguelino requested a review from Copilot March 26, 2025 16:00
Copy link
Copy Markdown

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.

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

@nicomiguelino nicomiguelino requested a review from Copilot March 26, 2025 16:02
Copy link
Copy Markdown

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 enforces stricter TypeScript type safety by disallowing the use of "any" and updating function signatures and ESLint configuration accordingly. Key changes include:

  • Updating .eslintrc.cjs to use TypeScript-specific rules and setting explicit error levels for prohibited types.
  • Refactoring various functions across utils, actions, and triggers to replace "any" with proper types.
  • Adjusting test expectations to use the new response property ("data") instead of "json".

Reviewed Changes

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

Show a summary per file
File Description
.eslintrc.cjs Updated ESLint configuration for TypeScript and test environments
src/types/screenly.ts Added TypeScript interface definitions for Screenly API types
test/* Modified tests to expect "data" instead of "json" responses
src/utils.ts Refactored helper functions with strict types and removed the unused makeRequest
src/actions/* and src/triggers/* Converted functions to use explicit ZObject and Bundle types
Files not reviewed (1)
  • package.json: Language not supported
Comments suppressed due to low confidence (2)

src/actions/complete-workflow.ts:95

  • The code is still accessing the 'json' property while other parts use 'data' after handleError modifications. Please update this line to use 'labelQueryResponse.data' for consistency.
const existingLabels = labelQueryResponse.json;

src/utils.ts:75

  • Using a falsy check for 'duration' may unintentionally skip valid values such as 0. Consider checking explicitly against undefined (e.g., 'if (duration !== undefined)') if 0 is a valid duration.
if (duration) {

@nicomiguelino nicomiguelino marked this pull request as ready for review March 26, 2025 16:12
@nicomiguelino nicomiguelino merged commit d63edba into Screenly:master Mar 26, 2025
5 checks passed
@nicomiguelino nicomiguelino deleted the disallow-any branch March 26, 2025 16:21
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.

3 participants