Upgrade eslint-plugin-primer-react to v8.4.0 and enable spread-props-first rule: warn

[Copilot]

Changelog

• Updated eslint-plugin-primer-react from ^8.2.0 to ^8.4.0
• Added primer-react/spread-props-first: warn (rather than error for incremental adoption)
• Removed --max-warnings=0 from the npm run lint to allow warnings in CI again (added 4 years ago)

Rollout strategy

• Patch release
• Minor release
• Major release; if selected, include a written rollout or migration plan
• None; if selected, internal eslint change only

Testing & Reviewing

Merge checklist

• NA Added/updated tests
• NA Added/updated documentation
• NA Added/updated previews (Storybook)
• NA Changes are SSR compatible
• NA Tested in Chrome
• NA Tested in Firefox
• NA Tested in Safari
• NA Tested in Edge
• NA (GitHub staff only) Integration tests pass at github/github (Learn more about how to run integration tests)

[changeset-bot]

⚠️ No Changeset found

Latest commit: 2d9729f

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

[siddharthkp]

@copilot Once you are done, also run npm run lint --fix

[Copilot]

@copilot Once you are done, also run npm run lint --fix

Done! npm run lint --fix completed with no changes needed - the code is already properly linted.

[github-actions]

👋 Hi, this pull request contains changes to the source code that github/github depends on. If you are GitHub staff, we recommend testing these changes with github/github using the integration workflow. Thanks!

[siddharthkp]

@joshblack 159 files changed 😅

Do we rip the bandaid and update all of them? Alternate would be to make it a "suggestion" instead of "error" and over time whenever someone touches a file, they'd be encouraged to update that file?

Also, Wow didn't expect so many tests to fail as well!

[joshblack]

@siddharthkp I think we'll want this to be more incremental (whatever batch size makes sense) because we might see the issue come up where someone used to pass something that worked and now it won't if we have the same prop (e.g. className, on* props, whatever). What do you think?

[siddharthkp]

we might see the issue come up where someone used to pass something that worked and now it won't if we have the same prop

Makes sense, definitely expecting that!

[siddharthkp]

@copilot Move the rule to "suggestion" instead of "error". Remove all of the lint fixes that you had performed

[Copilot]

@copilot Move the rule to "suggestion" instead of "error". Remove all of the lint fixes that you had performed

Done! Changed the rule to 'warn' and reverted all lint fixes in commit 0469266.

The rule now shows 321 warnings across the codebase that can be fixed incrementally. Note: npm run lint will currently fail due to --max-warnings=0 - you may want to adjust this temporarily during the migration period.

[Copilot]

Pull Request Overview

This PR upgrades the eslint-plugin-primer-react package to v8.4.0 and enables the new spread-props-first ESLint rule as a warning to prevent unintentional prop overrides when spread operators are used after named props.

Key changes:

• Package version bump from ^8.2.0 to ^8.4.0
• New ESLint rule enabled as 'warn' to allow incremental adoption
• Removed --max-warnings=0 flag from lint script to allow CI to pass with warnings

Reviewed Changes

Copilot reviewed 2 out of 3 changed files in this pull request and generated no comments.

File	Description
package.json	Updated eslint-plugin-primer-react version and modified lint script to allow warnings
eslint.config.mjs	Added spread-props-first rule configuration as warning level

[siddharthkp]

Removed integration-tests: recommended because now there are no changes to any src files

[github-actions]

👋 Hi, this pull request contains changes to the source code that github/github depends on. If you are GitHub staff, we recommend testing these changes with github/github using the integration workflow. Thanks!

[siddharthkp]

Note for reviewer:

We could add a number here which matches the current number of warnings for this one rule, but then we also have to keep it updated any time we fix a warning so that other warnings don't sneak into the gap

Suggested change

	"lint": "eslint '**/*.{js,ts,tsx,md,mdx}' --cache",
	"lint": "eslint '**/*.{js,ts,tsx,md,mdx}' --max-warnings=321 --cache",

[siddharthkp]

Note for reviewer: This is the only bad part about making the rule "warn" instead of "error". We open up warnings from other rules as well because there is no way to allow warnings from one rule.

[siddharthkp]

Added my comments for other reviewers

[joshblack]

@siddharthkp just wanted to share an idea for the migration if it would be of interest (also sorry about the delay!)

We could have the rule off by default but only enable it for a certain number of files (or exclude files where we know it's a problem) and could use the migration-status workflow to report on the migration: https://github.com/primer/react/blob/main/.github/workflows/migration-status.yml

So we could:

1. Enable it by default (so that new files have the rule enabled)
2. Add an override in the eslint config that turns it off for the files that we want to change over time (this can be what is used in the migraiton script)
3. Instruct copilot to work in batches based off of that override, gradually moving over until there are no more overrides

This could help to prevent the warn vs error kind of stuff too if it's helpful