Changes the signature of spfx rules to accepts object as args. Closes #7067#7082
Open
waldekmastykarz wants to merge 1 commit intopnp:mainfrom
Open
Changes the signature of spfx rules to accepts object as args. Closes #7067#7082waldekmastykarz wants to merge 1 commit intopnp:mainfrom
waldekmastykarz wants to merge 1 commit intopnp:mainfrom
Conversation
There was a problem hiding this comment.
Pull request overview
This PR refactors the constructor signatures of SPFx upgrade and doctor rules to accept object parameters instead of individual arguments, improving code maintainability and flexibility.
Changes:
- Updated base rule classes (
DependencyRule,ResolutionRule,PackageRule,FileAddRemoveRule) to accept configuration objects - Modified all derived rule classes to use the new object-based constructor pattern
- Updated test files and doctor configuration files to instantiate rules with object parameters
Reviewed changes
Copilot reviewed 300 out of 346 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
src/m365/spfx/commands/project/project-upgrade/rules/DependencyRule.ts |
Introduced DependencyRuleOptions interface and refactored constructor to accept options object |
src/m365/spfx/commands/project/project-upgrade/rules/ResolutionRule.ts |
Changed constructor to accept object with packageName and packageVersion properties |
src/m365/spfx/commands/project/project-upgrade/rules/PackageRule.ts |
Updated constructor to accept object with propertyName, add, and optional propertyValue |
src/m365/spfx/commands/project/project-upgrade/rules/FileAddRemoveRule.ts |
Modified constructor to accept object with filePath, add, and optional contents |
src/m365/spfx/commands/project/project-doctor/rules/DependencyRule.ts |
Introduced DependencyRuleOptions interface with packageName, supportedRange, and optional isDevDep |
| Multiple FN*.ts files | Updated rule instantiation to use object parameters with spread syntax where applicable |
| Multiple spec files | Updated test instantiation to use object-based constructor calls |
| Multiple doctor-*.ts files | Updated rule instantiation in doctor configurations to use object parameters |
Comments suppressed due to low confidence (2)
src/m365/spfx/commands/project/project-doctor/doctor-1.22.0.ts:1
- The property name should be
supportedRangenotpackageVersionbased on theFN002021_DEVDEP_rushstack_eslint_configconstructor signature which expects{ supportedRange: string }.
src/m365/spfx/commands/project/project-doctor/doctor-1.21.0.ts:1 - The property name should be
supportedRangenotpackageVersionbased on theFN002021_DEVDEP_rushstack_eslint_configconstructor signature which expects{ supportedRange: string }.
src/m365/spfx/commands/project/project-doctor/rules/FN002021_DEVDEP_rushstack_eslint_config.ts
Show resolved
Hide resolved
| new FN002013_DEVDEP_types_webpack_env({ supportedRange: '~1.15.2' }), | ||
| new FN002015_DEVDEP_types_react({ supportedRange: '17' }), | ||
| new FN002016_DEVDEP_types_react_dom({ supportedRange: '17' }), | ||
| new FN002021_DEVDEP_rushstack_eslint_config({ packageVersion: '4.5.2' }), |
There was a problem hiding this comment.
The property name should be supportedRange not packageVersion based on the FN002021_DEVDEP_rushstack_eslint_config constructor signature which expects { supportedRange: string }.
Suggested change
| new FN002021_DEVDEP_rushstack_eslint_config({ packageVersion: '4.5.2' }), | |
| new FN002021_DEVDEP_rushstack_eslint_config({ supportedRange: '4.5.2' }), |
| new FN002015_DEVDEP_types_react({ supportedRange: '17' }), | ||
| new FN002016_DEVDEP_types_react_dom({ supportedRange: '17' }), | ||
| new FN002019_DEVDEP_microsoft_rush_stack_compiler({ supportedVersions: ['5.3'] }), | ||
| new FN002021_DEVDEP_rushstack_eslint_config({ packageVersion: '4.0.1' }), |
There was a problem hiding this comment.
The property name should be supportedRange not packageVersion based on the FN002021_DEVDEP_rushstack_eslint_config constructor signature which expects { supportedRange: string }.
Suggested change
| new FN002021_DEVDEP_rushstack_eslint_config({ packageVersion: '4.0.1' }), | |
| new FN002021_DEVDEP_rushstack_eslint_config({ supportedRange: '4.0.1' }), |
| new FN002015_DEVDEP_types_react({ supportedRange: '17' }), | ||
| new FN002016_DEVDEP_types_react_dom({ supportedRange: '17' }), | ||
| new FN002019_DEVDEP_microsoft_rush_stack_compiler({ supportedVersions: ['4.7'] }), | ||
| new FN002021_DEVDEP_rushstack_eslint_config({ packageVersion: '4.0.1' }), |
There was a problem hiding this comment.
The property name should be supportedRange not packageVersion based on the FN002021_DEVDEP_rushstack_eslint_config constructor signature which expects { supportedRange: string }.
Suggested change
| new FN002021_DEVDEP_rushstack_eslint_config({ packageVersion: '4.0.1' }), | |
| new FN002021_DEVDEP_rushstack_eslint_config({ supportedRange: '4.0.1' }), |
Contributor
There was a problem hiding this comment.
This should also be done for:
- doctor-1.21.0
- doctor-1.22.0
Jwaegebaert
requested changes
Feb 23, 2026
Contributor
Jwaegebaert
left a comment
Jwaegebaert
left a comment
There was a problem hiding this comment.
Nice job with this one. Nothing further but the feedback Copilot provided. Be sure to take a look at the remarks left by Copilot
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.
Changes the signature of spfx rules to accepts object as args. Closes #7067