ESLint v9.15.0 introduces default options using themeta.defaultOptions property. This PR updates the rules to adddefaultOptions property tometa object, and retains the previousdefaultOptions placement to ensure backward compatibility.

fix(utils): add defaultOptions to meta

a1cb096

Copy link

Contributor

typescript-eslintbot commentedNov 16, 2024

Thanks for the PR,@LuisUrrutia!

typescript-eslint is a 100% community driven project, and we are incredibly grateful that you are contributing to that community.

The core maintainers work on this in their personal time, so please understand that it may not be possible for them to review your work immediately.

Thanks again!

🙏Please, if you or your company is finding typescript-eslint valuable, help us sustain the project by sponsoring it transparently on https://opencollective.com/typescript-eslint.

Copy link

netlifybot commentedNov 16, 2024•
edited
Loading

✅ Deploy Preview fortypescript-eslint ready!

Name	Link
🔨 Latest commit	`0537ef6`
🔍 Latest deploy log	https://app.netlify.com/sites/typescript-eslint/deploys/673965bc1e8ea90008217d3a
😎 Deploy Preview	https://deploy-preview-10339--typescript-eslint.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.
Lighthouse	1 paths audited Performance: 98 (🔴 down 1 from production) Accessibility: 100 (no change from production) Best Practices: 92 (no change from production) SEO: 98 (no change from production) PWA: 80 (no change from production) View the detailed breakdown and full score reports

To edit notification comments on pull requests, go to yourNetlify site configuration.

Copy link

nx-cloudbot commentedNov 16, 2024•
edited
Loading

☁️ Nx Cloud Report

CI is running/has finished running commands for commit0537ef6. As they complete they will appear below. Click to see the status, the terminal output, and the build insights.

📂 See all runs for this CI Pipeline Execution

✅ Successfully ran 4 targets

Sent with 💌 fromNxCloud.

bradzacher requested changes

Nov 16, 2024

View reviewed changes

Copy link

Member

bradzacher left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

thanks for doing this!
a few comments but it's looking good so far!

packages/utils/src/ts-eslint/Rule.ts Outdated

Comment on lines 72 to 73


		defaultOptions?: Options;

Copy link

Member

bradzacherNov 16, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

All properties need JSDoc!

Suggested change


	defaultOptions?:Options;
	/**
	*Specifiesdefaultoptionsfortherule.Ifpresent, anyuser-providedoptionsintheirconfigwillbemergedontopofthemrecursively.
	*Thismergingwillbeapplieddirectlyto`context.options`.
	*Ifyouwantbackwards-compatiblesupportforearlierESLintversion;consider usingthetop-level`defaultOptions`instead.
	*
	* @sinceESLint9.15.0
	*/
	defaultOptions?:Options;

Copy link

ContributorAuthor

LuisUrrutiaNov 16, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Added, but@since can't be used withESLint word.

packages/utils/src/eslint-utils/RuleCreator.ts Outdated

Comment on lines 65 to 76

		defaultOptions,
		meta,
		name,
		...rule
		}: Readonly<
		RuleWithMetaAndName<Options, MessageIds, PluginDocs>
		>): RuleModule<MessageIds, Options, PluginDocs> {
		return createRule<Options, MessageIds, PluginDocs>({
		defaultOptions,
		meta: {
		...meta,
		defaultOptions,

Copy link

Member

bradzacherNov 16, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

It's worth noting that adding this to this function will fix the bug with base rule extensions because our codebase uses theRuleCreator function -- but it might leave the broader ecosystem in a weird state.
This will cause all rules to have a differentcontext.options value when run under ESLint v9.15.0 -- which could cause breakages in the ecosystem.

For reference -- currently we do not modify thecontext.options value -- meaning it exactly mirrors the value that a user sets in their config file and we instead pass the "merged options" in via a second argument --create(context, mergedOptions).
So if you passmeta.defaultOptions automatically then the first part of this behaviour no longer holds true and insteadcontext.options will represent the "merged options".

For now we should leave this function as-is, and instead update our extension rule implementation to passdefaultOptions both in the top-level location (for our current rule logic) and in themeta.defaultOptions location (for ESLint base rule logic).

packages/utils/src/eslint-utils/RuleCreator.ts Outdated

		@@ -13,10 +13,11 @@ export type NamedCreateRuleMetaDocs = Omit<RuleMetaDataDocs, 'url'>;

		export type NamedCreateRuleMeta<
		MessageIds extends string,
		Options extends readonly unknown[],

Copy link

Member

bradzacherNov 16, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Adding this new generic in this location is a breaking change.
It changes the meaning of existing code and could introduce type errors.
Instead it must be added after all type parameters AND it must be given a default value of[]

bradzacher added bug

Something isn't working

awaiting responseIssues waiting for a reply from the OP or another party labels

Nov 16, 2024

LuisUrrutia added2 commits

November 16, 2024 09:02

fix(utils): add defaultOptions to rule instead of RuleCreator

59a55f5

fix(utils): add defaultOptions jsdocs

d5a9b16

LuisUrrutia changed the title~~fix(utils): add defaultOptions to meta~~fix(utils): add defaultOptions to meta in rule

Nov 16, 2024

Copy link

codecovbot commentedNov 16, 2024•
edited
Loading

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 86.69%. Comparing base(57d343b) to head(0537ef6).
Report is 12 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@##             main   #10339      +/-   ##==========================================+ Coverage   86.63%   86.69%   +0.06%==========================================  Files         433      434       +1       Lines       15202    15227      +25       Branches     4439     4445       +6     ==========================================+ Hits        13170    13201      +31+ Misses       1675     1670       -5+ Partials      357      356       -1

Flag	Coverage Δ
unittest	`86.69% <100.00%> (+0.06%)`	⬆️

Flags with carried forward coverage won't be shown.Click here to find out more.

Files with missing lines	Coverage Δ
...kages/eslint-plugin/src/rules/consistent-return.ts	`93.75% <100.00%> (+0.13%)`	⬆️
packages/eslint-plugin/src/rules/dot-notation.ts	`96.66% <100.00%> (+0.11%)`	⬆️
...kages/eslint-plugin/src/rules/init-declarations.ts	`100.00% <ø> (ø)`
packages/eslint-plugin/src/rules/max-params.ts	`94.44% <ø> (ø)`
...s/eslint-plugin/src/rules/no-dupe-class-members.ts	`100.00% <ø> (ø)`
...kages/eslint-plugin/src/rules/no-empty-function.ts	`91.42% <100.00%> (+0.25%)`	⬆️
...ackages/eslint-plugin/src/rules/no-invalid-this.ts	`100.00% <100.00%> (ø)`
packages/eslint-plugin/src/rules/no-loop-func.ts	`93.93% <ø> (ø)`
...es/eslint-plugin/src/rules/no-loss-of-precision.ts	`100.00% <ø> (ø)`
...ckages/eslint-plugin/src/rules/no-magic-numbers.ts	`84.72% <ø> (ø)`
... and5 more

... and11 files with indirect coverage changes

Copy link

ContributorAuthor

LuisUrrutia commentedNov 16, 2024

@bradzacher I made the requested changes, but now I will add thedefaultOptions to other rules.

refactor: add defaultOptions to affected eslint rules

49bac3f

LuisUrrutia requested a review frombradzacher

November 16, 2024 14:50

github-actionsbot removed the awaiting responseIssues waiting for a reply from the OP or another party label

Nov 16, 2024

mdjermanovic mentioned this pull request

Nov 16, 2024

fix: fix getting options in dot-notation ruleeslint/eslint#19140

Closed

9 tasks

bradzacher added14 commits

November 16, 2024 19:35

Update init-declarations.ts

3eca8be

Update max-params.ts

76b40be

Update no-dupe-class-members.ts

da04432

Update init-declarations.ts

097e978

Update max-params.ts

8bda23c

Update no-loop-func.ts

40acfda

Update no-loss-of-precision.ts

d1eb53e

Update no-magic-numbers.ts

6ec4f78

Discard changes to packages/eslint-plugin/src/rules/no-restricted-imp…

2da4fc4

…orts.ts

Update no-restricted-imports.ts

54cd570

Discard changes to packages/eslint-plugin/src/rules/no-useless-constr…

28cc05b

…uctor.ts

Update no-useless-constructor.ts

c88a353

Discard changes to packages/eslint-plugin/src/rules/prefer-destructur…

adc19f4

…ing.ts

Update prefer-destructuring.ts

0537ef6

Copy link

Member

bradzacher commentedNov 17, 2024•
edited by kirkwaiblinger
Loading

Sorry about the commit spam.
I tested your PR against 9.15 and found that it introduced some unexpected breakages.
You had applied thedefaultOptions refactoring to every extension rule.
But this broke certain rules (egprefer-destructuring) due to their more complex schemas!

I just reverted some of the changes you made to limit them to exactly the rules touched ineslint/eslint#17656

bradzacher approved these changes

Nov 17, 2024

View reviewed changes

Copy link

Member

bradzacher left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

thanks for doing this!
I have tested this against 9.15 and this looks good!

bradzacher merged commitf5e23e2 intotypescript-eslint:main

Nov 17, 2024

62 checks passed

EduardoAC added a commit to EduardoAC/snap-components that referenced this pull request

Nov 17, 2024

Fix regression caused by typescript-eslint in combination with eslint…

84f6bb6

…@9.15.0Details on:typescript-eslint/typescript-eslint#10339

EduardoAC mentioned this pull request

Nov 17, 2024

Setup GitHub Actions for Pull request and main branchEduardoAC/snap-components#4

Merged

EduardoAC added a commit to EduardoAC/snap-components that referenced this pull request

Nov 17, 2024

Setup GitHub Actions for Pull request and main branch (#4)

39a5325

* Setup GitHub Actions for Pull request and main branch* Fix regression caused by typescript-eslint in combination with eslint@9.15.0Details on:typescript-eslint/typescript-eslint#10339* Resolve linter issues after updating packages

xen0n mentioned this pull request

Nov 18, 2024

build(deps-dev): bump eslint from 9.14.0 to 9.15.0loongson-community/areweloongyet#234

Closed

Copy link

pavelkornev commentedNov 18, 2024

When do you plan to release this fix?

Copy link

Member

bradzacher commentedNov 18, 2024

Please see the issue linked in the OP

typescript-eslint locked and limited conversation to collaborators

Nov 18, 2024

Labels

bug

Something isn't working

3 participants

Movatterモバイル変換

Uh oh!

fix(utils): add defaultOptions to meta in rule#10339

fix(utils): add defaultOptions to meta in rule#10339

Uh oh!

Conversation

LuisUrrutia commentedNov 16, 2024• editedLoading Uh oh!There was an error while loading.Please reload this page.

Uh oh!

PR Checklist

Overview

Uh oh!

typescript-eslintbot commentedNov 16, 2024

Uh oh!

netlifybot commentedNov 16, 2024• editedLoading Uh oh!There was an error while loading.Please reload this page.

Uh oh!

✅ Deploy Preview fortypescript-eslint ready!

Uh oh!

nx-cloudbot commentedNov 16, 2024• editedLoading Uh oh!There was an error while loading.Please reload this page.

Uh oh!

☁️ Nx Cloud Report

Uh oh!

bradzacher left a comment

Choose a reason for hiding this comment

Uh oh!

bradzacherNov 16, 2024

Choose a reason for hiding this comment

Uh oh!

LuisUrrutiaNov 16, 2024

Choose a reason for hiding this comment

Uh oh!

bradzacherNov 16, 2024

Choose a reason for hiding this comment

Uh oh!

bradzacherNov 16, 2024

Choose a reason for hiding this comment

Uh oh!

codecovbot commentedNov 16, 2024• editedLoading Uh oh!There was an error while loading.Please reload this page.

Uh oh!

Codecov Report

Uh oh!

LuisUrrutia commentedNov 16, 2024

Uh oh!

bradzacher commentedNov 17, 2024• edited by kirkwaiblingerLoading Uh oh!There was an error while loading.Please reload this page.

Uh oh!

Uh oh!

bradzacher left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

pavelkornev commentedNov 18, 2024

Uh oh!

bradzacher commentedNov 18, 2024

Uh oh!

Uh oh!

LuisUrrutia commentedNov 16, 2024•
edited
Loading

netlifybot commentedNov 16, 2024•
edited
Loading

nx-cloudbot commentedNov 16, 2024•
edited
Loading

codecovbot commentedNov 16, 2024•
edited
Loading

bradzacher commentedNov 17, 2024•
edited by kirkwaiblinger
Loading