refactor: remove deprecated widget prop types, eliminate all deprecated code, and consolidate BaseWidgetProps for v1.0#693
Conversation
…entations Co-authored-by: pethers <1726836+pethers@users.noreply.github.com>
Co-authored-by: pethers <1726836+pethers@users.noreply.github.com>
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.Scanned FilesNone |
There was a problem hiding this comment.
Pull Request Overview
This PR refactors widget prop type definitions to remove deprecated properties, improve type safety, and eliminate ESLint violations for empty interfaces. The changes enforce the use of specific CIA level props (availabilityLevel, integrityLevel, confidentialityLevel) instead of the ambiguous generic level prop, while maintaining backward compatibility through deprecation notices.
Key Changes:
- Removed deprecated
levelprop from widget implementations and cleaned up interfaces - Converted empty interfaces to type aliases (CostEstimationWidgetProps, TechnicalDetailsWidgetProps)
- Marked duplicate interfaces in
widgets.tsas deprecated with migration paths towidget-props.ts
Reviewed Changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/types/widgets.ts | Added deprecation notices to ComponentImpactBaseProps and impact widget types; converted CostEstimationWidgetProps to type alias; streamlined documentation |
| src/types/widget-props.ts | Removed deprecated level props from ComponentImpactBaseProps and three impact widget interfaces; converted TechnicalDetailsWidgetProps to type alias; updated documentation |
| src/components/widgets/impactanalysis/IntegrityImpactWidget.tsx | Removed deprecated level prop from component destructuring and logic; updated import to use widget-props.ts |
| src/components/widgets/impactanalysis/ConfidentialityImpactWidget.tsx | Removed deprecated level prop; changed destructured parameter from onLevelChange to onError; updated import to use widget-props.ts |
| src/components/widgets/impactanalysis/AvailabilityImpactWidget.tsx | Removed deprecated level prop and local interface definition in favor of imported type from widget-props.ts |
| src/components/widgets/impactanalysis/IntegrityImpactWidget.test.tsx | Updated test names and removed deprecated level prop usage from one test case; one test still uses deprecated prop |
| src/components/widgets/impactanalysis/ConfidentialityImpactWidget.test.tsx | Updated test names and removed deprecated level prop from test cases |
| src/components/widgets/impactanalysis/AvailabilityImpactWidget.test.tsx | Updated test names and removed deprecated level prop from test cases |
| className = "", | ||
| testId = "widget-confidentiality-impact", | ||
| onLevelChange: _onLevelChange, | ||
| onError: _onError, |
There was a problem hiding this comment.
The destructured parameter name was changed from onLevelChange to onError, but this creates an inconsistency:
- The deprecated
ComponentImpactBasePropsinwidgets.ts(line 536) hasonLevelChange?: (level: SecurityLevel) => void - The new
ConfidentialityImpactWidgetPropsinwidget-props.tsdoesn't haveonLevelChangebut hasonErrorfromBaseWidgetProps
This breaks backward compatibility with code using the deprecated interface. Since the deprecated types in widgets.ts are type aliases pointing to ComponentImpactBaseProps which has onLevelChange, existing consumers could be passing this prop.
Recommendation: Either:
- Add
onLevelChange?: (level: SecurityLevel) => voidto all three widget prop interfaces inwidget-props.tsto maintain backward compatibility, OR - Keep destructuring
onLevelChange(even if unused) to avoid TypeScript errors when using deprecated interfaces
Note: The same issue exists in IntegrityImpactWidget and AvailabilityImpactWidget which don't destructure onLevelChange either.
| onError: _onError, | |
| onError: _onError, | |
| onLevelChange, // for backward compatibility with deprecated interface |
There was a problem hiding this comment.
The current implementation correctly matches the new ConfidentialityImpactWidgetProps interface from widget-props.ts, which intentionally does not include onLevelChange.
The deprecated ComponentImpactBaseProps in widgets.ts is marked for removal post-v1.0 and serves to show deprecation warnings, not maintain full backward compatibility. The component uses onError from BaseWidgetProps which is the correct callback for error handling in the new type system.
Since the component is explicitly typed with the new interface and TypeScript compilation succeeds without errors, adding onLevelChange would actually be inconsistent with the declared interface. Developers using the deprecated types will receive deprecation warnings guiding them to migrate to the new interfaces.
| className = "", | ||
| testId = "widget-confidentiality-impact", | ||
| onLevelChange: _onLevelChange, | ||
| onError: _onError, |
There was a problem hiding this comment.
The showExtendedDetails prop is missing from the destructured parameters, but it's defined in the ConfidentialityImpactWidgetProps interface (line 728 in widget-props.ts).
For consistency with the other two impact widgets:
IntegrityImpactWidgetdestructuresshowExtendedDetails = false(line 32)AvailabilityImpactWidgetdestructuresshowExtendedDetails: _showExtendedDetails = false(line 36)
Recommendation: Add showExtendedDetails to the destructured parameters (prefixed with _ if unused) to maintain consistency and avoid potential TypeScript errors when consumers pass this valid prop.
| onError: _onError, | |
| onError: _onError, | |
| showExtendedDetails: _showExtendedDetails = false, |
There was a problem hiding this comment.
Fixed in commit 3f1311b. Added showExtendedDetails: _showExtendedDetails = false to the destructured parameters for consistency with the other impact widgets.
|
@copilot apply changes based on the comments in this thread |
Co-authored-by: pethers <1726836+pethers@users.noreply.github.com>
|
@copilot want release 1.0 to contain no deprecated code/methods time to remove them all before 1.0 (no users yet) |
Co-authored-by: pethers <1726836+pethers@users.noreply.github.com>
Completed in commit 19d11c9. Removed all deprecated code/methods for v1.0: Deprecated Types Removed:
Deprecated Functions Removed:
Results:
|
| // Export remaining widget types from widgets.ts | ||
| export type { | ||
| WidgetProps, | ||
| } from "./widgets"; |
There was a problem hiding this comment.
There are two different BaseWidgetProps interfaces defined in the codebase:
- In
widgets.ts(line 31): ExtendsCommonWidgetPropsand addschildren?: ReactNode - In
widget-props.ts(line 239): ExtendsCommonWidgetPropsand addsonError?: (error: Error) => void
Line 80 exports BaseWidgetProps from widgets.ts, but the impact widgets (AvailabilityImpactWidgetProps, IntegrityImpactWidgetProps, ConfidentialityImpactWidgetProps) extend BaseWidgetProps from widget-props.ts (which has onError). This creates naming conflicts and ambiguity about which BaseWidgetProps should be used.
Recommendation: Consolidate into a single BaseWidgetProps interface that includes both children and onError properties, or rename one of them to be more specific (e.g., BaseWidgetPropsWithChildren or ErrorHandlingWidgetProps).
There was a problem hiding this comment.
Fixed in commit 0d18c5d. Consolidated the duplicate BaseWidgetProps interfaces into a single definition in widget-props.ts that includes both children and onError properties. Removed the duplicate from widgets.ts and updated all exports to use the canonical version, eliminating naming conflicts.
| testId = "widget-confidentiality-impact", | ||
| onLevelChange: _onLevelChange, | ||
| showExtendedDetails: _showExtendedDetails = false, | ||
| onError: _onError, |
There was a problem hiding this comment.
The onError prop is destructured but not used in the component (prefixed with _). Unlike showExtendedDetails, this prop is inherited from BaseWidgetProps and should be utilized for error handling consistency across all widgets.
The widget does have error handling via the useCIAContentService hook which provides a serviceError, but this error is only passed to the WidgetContainer component. The onError callback should be invoked when errors occur to allow parent components to handle them. Consider adding error handling like:
useEffect(() => {
if (serviceError && _onError) {
_onError(serviceError);
}
}, [serviceError, _onError]);This would make error handling consistent with the component's API contract defined by BaseWidgetProps.
There was a problem hiding this comment.
Fixed in commit 0d18c5d. Added useEffect hook to invoke the onError callback when serviceError occurs, making error handling consistent with the component's API contract defined by BaseWidgetProps.
|
@copilot apply changes based on the comments in this thread |
Co-authored-by: pethers <1726836+pethers@users.noreply.github.com>
…tion or class' Co-authored-by: Copilot Autofix powered by AI <223894421+github-code-quality[bot]@users.noreply.github.com> Signed-off-by: James Pether Sörling <pethers@users.noreply.github.com>
🔧 Fix widget prop type inconsistencies and eliminate any types ✅
Summary
Successfully eliminated all
anytypes and fixed widget prop type inconsistencies to ensure strict TypeScript compliance for v1.0 release. All deprecated code removed for v1.0 - no users yet, clean slate achieved. Consolidated duplicateBaseWidgetPropsinterfaces and implemented consistent error handling across widgets.Recent Updates
BaseWidgetPropsinterfaces into single source of truthConfidentialityImpactWidgetfor consistent error propagationConfidentialityImpactWidgetto properly destructureshowExtendedDetailsprop for consistency with other impact widgetsAudit Results
anytypes found in 265 non-test TypeScript filesBaseWidgetPropsdefinitionCompleted Tasks
anytypes - RESULT: 0 foundanywith proper TypeScript typessrc/types/widget-props.tstsc --noEmitsuccessfully with strict checksChanges Made
1. Removed Deprecated Props ✅
levelprop fromComponentImpactBasePropsinwidget-props.tslevelprop from all impact widget interfaces:AvailabilityImpactWidgetPropsIntegrityImpactWidgetPropsConfidentialityImpactWidgetProps2. Updated Widget Implementations ✅
AvailabilityImpactWidget: Removed deprecated prop handling, uses onlyavailabilityLevelIntegrityImpactWidget: Removed deprecated prop handling, uses onlyintegrityLevelConfidentialityImpactWidget:confidentialityLevelshowExtendedDetailspropuseEffecthook3. Fixed ESLint Issues ✅
@typescript-eslint/no-empty-object-typeviolations4. Improved Type Consistency ✅
widget-props.tsBaseWidgetPropsinterfaces:BaseWidgetPropsfromwidgets.ts(withchildren) andwidget-props.ts(withonError) into single definitionchildren?: ReactNodeandonError?: (error: Error) => voidwidget-props.ts5. Removed All Deprecated Code ✅
Deprecated Types Removed:
ComponentImpactBaseProps,AvailabilityImpactWidgetProps,IntegrityImpactWidgetProps,ConfidentialityImpactWidgetPropsfromwidgets.tsEnhancedCIADetailsfromcia-services.tsgetValuePointsfromCIADataProviderinterfaceBaseWidgetPropsfromwidgets.tsDeprecated Functions Removed:
calculateImplementationCostandcalculateOperationalCostfrombusinessValueUtils.tsformatSecurityLevelandSecurityLevelDisplayfromutils/index.tsCleaned Up:
index.tsto use canonical sourcesgetValuePointsalias6. Updated Tests ✅
7. Enhanced Error Handling ✅
useEffecthook inConfidentialityImpactWidgetto invokeonErrorcallbackType Safety Metrics
anytypes (non-test)Build & Test Status
Files Modified
src/types/widget-props.ts- Removed deprecated props, consolidated BaseWidgetProps, added ReactNode importsrc/types/widgets.ts- Removed duplicate BaseWidgetProps and deprecated interfaces, updated importssrc/types/cia-services.ts- Removed deprecated typessrc/types/index.ts- Updated exports to use canonical sources from widget-propssrc/utils/businessValueUtils.ts- Removed deprecated functionssrc/utils/index.ts- Removed deprecated exportssrc/utils/riskUtils.ts- Removed deprecation noticesrc/data/ciaOptionsData.ts- Removed deprecated aliassrc/components/widgets/impactanalysis/ConfidentialityImpactWidget.tsx- Updated implementation, added error handling with useEffect hooksrc/components/widgets/impactanalysis/*.tsx- Updated implementations (2 other files)Impact
Benefits:
Breaking Changes:
Technical Details
BaseWidgetProps Consolidation:
The duplicate
BaseWidgetPropsinterfaces have been merged into a single canonical definition inwidget-props.ts:Error Handling Pattern:
Added consistent error handling in widgets:
Migration Guide (for future reference)
All deprecated code has been removed. Codebase now uses:
BaseWidgetPropsfromwidget-props.ts(notwidgets.ts)widget-props.tsinstead ofwidgets.tsCIADetailsinstead ofEnhancedCIADetailscalculateTotalSecurityCostfromcostCalculationUtils.tsinstead of deprecated functionsgetDefaultValuePointsinstead ofgetValuePointsavailabilityLevel, etc.) not genericlevelonErrorcallback to handle widget errors properlyRelated Issue: Fixes widget prop type inconsistencies and eliminates any types + removes all deprecated code
Release Target: v1.0 - Clean slate achieved ✨
Original prompt
💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.