Conversation
* Initial plan * Convert 5 simple action files to TypeScript: globalNotification, modal, currentTrack, pendingCloses, revisions Co-authored-by: edisile <4310497+edisile@users.noreply.github.com> * Convert medium complexity action files to TypeScript: branches, architectures, availableRevisionsSelect, channelMap, history Co-authored-by: edisile <4310497+edisile@users.noreply.github.com> * Convert final complex action files to TypeScript: defaultTrack and pendingReleases Co-authored-by: edisile <4310497+edisile@users.noreply.github.com> * Fix test failure: remove extra properties from Progressive object Co-authored-by: edisile <4310497+edisile@users.noreply.github.com> * fix: available revisions action * fix: release response types * fix: types that copilot missed or mangled * fix: tests imports, no idea how they passed before * chore: cleanup unused imports * chore: types for gaEventTracking action --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: edisile <4310497+edisile@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR migrates Redux actions and tests from JavaScript to TypeScript, improving type safety throughout the releases management system. The refactoring moves action type definitions from reducer files to dedicated action files, establishing a cleaner architectural pattern. However, the migration introduces several type safety issues through excessive use of 'any' casts and incomplete type definitions that undermine the benefits of TypeScript.
Key changes:
- Converted JavaScript action files to TypeScript with proper type definitions
- Moved action type definitions from reducer files to action files for better organization
- Updated all reducer and test imports to reference action files instead of reducer files
- Added type safety improvements to component props using typeof for function types
Reviewed changes
Copilot reviewed 52 out of 53 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| static/js/publisher/types/releaseTypes.ts | Updated AvailableRevisionsSelect to use typeof constants; exported ReleaseErrorResponse; modified DispatchFn type |
| static/js/publisher/pages/Releases/reducers/*.ts | Removed action type definitions and updated imports to reference action files |
| static/js/publisher/pages/Releases/reducers/tests/*.test.ts | Updated imports to reference action files; added type assertions to test data |
| static/js/publisher/pages/Releases/actions/*.ts | New TypeScript versions of action files with type definitions and action creators |
| static/js/publisher/pages/Releases/actions/*.js | Deleted JavaScript versions replaced by TypeScript files |
| static/js/publisher/pages/Releases/actions/index.ts | New barrel export file for centralized action exports |
| static/js/publisher/pages/Releases/components/*.tsx | Improved type safety for triggerGAEvent props using typeof and Parameters utility type |
Comments suppressed due to low confidence (7)
static/js/publisher/pages/Releases/reducers/pendingCloses.ts:21
- The reducer file exports 'ReleaseRevisionAction' and 'CancelPendingReleasesAction' which are actions that belong to pendingReleases, not pendingCloses. Following the refactoring pattern in this PR, these action types should only be defined and exported from their corresponding action files. The reducer should only import these types, not re-export them. This creates confusion and duplication.
static/js/publisher/pages/Releases/actions/defaultTrack.ts:108 - Multiple dispatch calls are being cast to 'any' to bypass type checking (lines 59, 95, 108). This undermines the type safety improvements of this PR. Instead of casting to 'any', consider updating the DispatchFn type or creating properly typed dispatch methods that accept the specific action types being used.
static/js/publisher/pages/Releases/actions/defaultTrack.ts:34 - Using '(window as any).CSRF_TOKEN' bypasses type checking. Consider defining a proper type declaration for the window object with CSRF_TOKEN property, such as: 'declare global { interface Window { CSRF_TOKEN: string; } }' in a types file.
static/js/publisher/pages/Releases/actions/pendingReleases.ts:135 - The object is being cast to 'Progressive' but only contains 'percentage' and 'paused'. The Progressive type requires three properties: "current-percentage", "paused", and "percentage" (all can be null). You're missing "current-percentage". Either provide all required Progressive properties or create a more specific type that only requires the properties being set.
static/js/publisher/pages/Releases/actions/history.ts:61 - The dispatch call is being cast to 'any' to bypass type checking. This defeats the purpose of adding TypeScript types. The issue is that 'closeHistory()' returns 'CloseHistoryAction' but 'DispatchFn' expects a union that includes 'GenericReleasesAction<string, never>'. Consider updating the DispatchFn type definition or the action union types to properly include all valid actions without requiring type casts.
static/js/publisher/pages/Releases/actions/pendingReleases.ts:157 - The payload is being cast to 'Progressive' but only contains 'percentage'. The Progressive type requires three properties: "current-percentage", "paused", and "percentage" (all can be null). You're missing "current-percentage". Either provide all required Progressive properties or create a more specific action payload type that only requires the properties being set.
static/js/publisher/pages/Releases/actions/pendingReleases.ts:168 - The payload is being cast to 'Progressive' but only contains 'percentage'. The Progressive type requires three properties: "current-percentage", "paused", and "percentage" (all can be null). You're missing "current-percentage". Either provide all required Progressive properties or create a more specific action payload type that only requires the properties being set.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Done
How to QA
no need for QA
Testing
Security
Issue / Card
Fixes #
Screenshots