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

LuisUrrutia · 2024-11-16T01:41:42Z

PR Checklist

Addresses an existing open issue: fixes Bug: [no-unused-expressions, no-empty-functions, possibly others...] extension rules crash with eslint v9.15.0 #10338
That issue was marked as accepting prs
Steps in Contributing were taken

Overview

ESLint v9.15.0 introduces default options using the meta.defaultOptions property. This PR updates the rules to add defaultOptions property to meta object, and retains the previous defaultOptions placement to ensure backward compatibility.

typescript-eslint · 2024-11-16T01:41:44Z

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.

netlify · 2024-11-16T01:41:56Z

✅ Deploy Preview for typescript-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 your Netlify site configuration.

nx-cloud · 2024-11-16T01:42:41Z

☁️ Nx Cloud Report

CI is running/has finished running commands for commit 0537ef6. 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 💌 from NxCloud.

bradzacher

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

bradzacher · 2024-11-16T02:14:31Z

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

+
+  defaultOptions?: Options;


All properties need JSDoc!

Suggested change

defaultOptions?: Options;

/**

* Specifies default options for the rule. If present, any user-provided options in their config will be merged on top of them recursively.

* This merging will be applied directly to `context.options`.

* If you want backwards-compatible support for earlier ESLint version; consider using the top-level `defaultOptions` instead.

*

* @since ESLint 9.15.0

*/

defaultOptions?: Options;

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

bradzacher · 2024-11-16T02:28:47Z

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

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


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

For reference -- currently we do not modify the context.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 pass meta.defaultOptions automatically then the first part of this behaviour no longer holds true and instead context.options will represent the "merged options".

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

bradzacher · 2024-11-16T02:31:55Z

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

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

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


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 []

codecov · 2024-11-16T12:34:08Z

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% <ø> (ø)`
... and 5 more

... and 11 files with indirect coverage changes

LuisUrrutia · 2024-11-16T12:41:27Z

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

…orts.ts

…uctor.ts

…ing.ts

bradzacher · 2024-11-17T03:42:53Z

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

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

bradzacher

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

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

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

pavelkornev · 2024-11-18T12:22:46Z

When do you plan to release this fix?

bradzacher · 2024-11-18T17:14:57Z

Please see the issue linked in the OP

fix(utils): add defaultOptions to meta

a1cb096

bradzacher requested changes Nov 16, 2024

View reviewed changes

bradzacher added bug Something isn't working awaiting response Issues waiting for a reply from the OP or another party labels Nov 16, 2024

LuisUrrutia added 2 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

refactor: add defaultOptions to affected eslint rules

49bac3f

LuisUrrutia requested a review from bradzacher November 16, 2024 14:50

github-actions bot removed the awaiting response Issues 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 rule eslint/eslint#19140

Closed

9 tasks

bradzacher added 14 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

bradzacher approved these changes Nov 17, 2024

View reviewed changes

bradzacher merged commit f5e23e2 into typescript-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.0 Details on: typescript-eslint/typescript-eslint#10339

EduardoAC mentioned this pull request Nov 17, 2024

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

Merged

xen0n mentioned this pull request Nov 18, 2024

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

Closed

typescript-eslint locked and limited conversation to collaborators Nov 18, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

LuisUrrutia commented Nov 16, 2024 •

edited

Loading

typescript-eslint bot commented Nov 16, 2024

netlify bot commented Nov 16, 2024 •

edited

Loading

nx-cloud bot commented Nov 16, 2024 •

edited

Loading

bradzacher left a comment

bradzacher Nov 16, 2024

LuisUrrutia Nov 16, 2024

bradzacher Nov 16, 2024

bradzacher Nov 16, 2024

codecov bot commented Nov 16, 2024 •

edited

Loading

LuisUrrutia commented Nov 16, 2024

bradzacher commented Nov 17, 2024 •

edited by kirkwaiblinger

Loading

bradzacher left a comment

pavelkornev commented Nov 18, 2024

bradzacher commented Nov 18, 2024

-  defaultOptions?: Options;
+  /**
+   * Specifies default options for the rule. If present, any user-provided options in their config will be merged on top of them recursively.
+   * This merging will be applied directly to `context.options`.
+   * If you want backwards-compatible support for earlier ESLint version; consider using the top-level `defaultOptions` instead.
+   *
+   * @since ESLint 9.15.0
+   */
+  defaultOptions?: Options;

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

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

Conversation

LuisUrrutia commented Nov 16, 2024 • edited Loading

PR Checklist

Overview

typescript-eslint bot commented Nov 16, 2024

netlify bot commented Nov 16, 2024 • edited Loading

✅ Deploy Preview for typescript-eslint ready!

nx-cloud bot commented Nov 16, 2024 • edited Loading

☁️ Nx Cloud Report

bradzacher left a comment

Choose a reason for hiding this comment

bradzacher Nov 16, 2024

Choose a reason for hiding this comment

LuisUrrutia Nov 16, 2024

Choose a reason for hiding this comment

bradzacher Nov 16, 2024

Choose a reason for hiding this comment

bradzacher Nov 16, 2024

Choose a reason for hiding this comment

codecov bot commented Nov 16, 2024 • edited Loading

Codecov Report

LuisUrrutia commented Nov 16, 2024

bradzacher commented Nov 17, 2024 • edited by kirkwaiblinger Loading

bradzacher left a comment

Choose a reason for hiding this comment

pavelkornev commented Nov 18, 2024

bradzacher commented Nov 18, 2024

LuisUrrutia commented Nov 16, 2024 •

edited

Loading

netlify bot commented Nov 16, 2024 •

edited

Loading

nx-cloud bot commented Nov 16, 2024 •

edited

Loading

codecov bot commented Nov 16, 2024 •

edited

Loading

bradzacher commented Nov 17, 2024 •

edited by kirkwaiblinger

Loading