Skip to content

ESLint-plugin: Disallow extra properties in eslint plugin rule options#32056

Merged
valentinpalkovic merged 5 commits intostorybookjs:nextfrom
andreww2012:fix/disallow-extra-properties-in-rule-options
Mar 10, 2026
Merged

ESLint-plugin: Disallow extra properties in eslint plugin rule options#32056
valentinpalkovic merged 5 commits intostorybookjs:nextfrom
andreww2012:fix/disallow-extra-properties-in-rule-options

Conversation

@andreww2012
Copy link
Copy Markdown
Contributor

@andreww2012 andreww2012 commented Jul 16, 2025

What I did

no-uninstalled-addons eslint-plugin-storybook rule currently allows extra properties to be passed in options object, which should not be allowed. This makes it easier for typos in rule options to go unnoticed.

This PR simply disallows extra properties in this rule's schema.

Checklist for Contributors

Testing

The changes in this PR are covered in the following automated tests:

  • stories
  • unit tests
  • integration tests
  • end-to-end tests

Manual testing

N/A

Documentation

  • Add or update documentation reflecting your changes
  • If you are deprecating/removing a feature, make sure to update
    MIGRATION.MD

Checklist for Maintainers

  • When this PR is ready for testing, make sure to add ci:normal, ci:merged or ci:daily GH label to it to run a specific set of sandboxes. The particular set of sandboxes can be found in code/lib/cli-storybook/src/sandbox-templates.ts

  • Make sure this PR contains one of the labels below:

    Available labels
    • bug: Internal changes that fixes incorrect behavior.
    • maintenance: User-facing maintenance tasks.
    • dependencies: Upgrading (sometimes downgrading) dependencies.
    • build: Internal-facing build tooling & test updates. Will not show up in release changelog.
    • cleanup: Minor cleanup style change. Will not show up in release changelog.
    • documentation: Documentation only changes. Will not show up in release changelog.
    • feature request: Introducing a new feature.
    • BREAKING CHANGE: Changes that break compatibility in some way with current major version.
    • other: Changes that don't fit in the above categories.

🦋 Canary release

This PR does not have a canary release associated. You can request a canary release of this pull request by mentioning the @storybookjs/core team here.

core team members can create a canary release here or locally with gh workflow run --repo storybookjs/storybook canary-release-pr.yml --field pr=<PR_NUMBER>

Greptile Summary

This PR adds additionalProperties: false to the schema configuration of the no-uninstalled-addons ESLint rule. This is a defensive programming practice that will help catch configuration errors early by preventing users from accidentally using invalid property names in their rule configuration.

The change affects how the rule validates its configuration options, now strictly enforcing that only packageJsonLocation and ignore are valid properties. Previously, if a user made a typo in their configuration (e.g., packgeJsonLocation instead of packageJsonLocation), the incorrect property would be silently ignored, potentially leading to confusing behavior.

Confidence score: 5/5

  1. This PR is extremely safe to merge as it only affects configuration validation
  2. The change is a straightforward schema update that follows ESLint best practices for rule configuration
  3. Files that need attention:
    • code/lib/eslint-plugin/src/rules/no-uninstalled-addons.ts

1 file reviewed, no comments
Edit PR Review Bot Settings | Greptile

Summary by CodeRabbit

  • Bug Fixes
    • Strengthened rule configuration validation by rejecting unknown options in settings, helping users identify configuration errors early and preventing silent failures.

✏️ Tip: You can customize this high-level summary in your review settings.

Copy link
Copy Markdown
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

Reviewing changes made in this pull request

@storybook-app-bot
Copy link
Copy Markdown

storybook-app-bot bot commented Jul 21, 2025

Package Benchmarks

Commit: 4733313, ran on 16 December 2025 at 07:39:45 UTC

No significant changes detected, all good. 👏

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Dec 15, 2025

📝 Walkthrough

Walkthrough

Added a schema constraint (additionalProperties: false) to the rule metadata in the ESLint plugin's no-uninstalled-addons rule, preventing unknown properties from being accepted in the rule's options object.

Changes

Cohort / File(s) Summary
ESLint rule schema constraint
code/lib/eslint-plugin/src/rules/no-uninstalled-addons.ts
Added additionalProperties: false to the rule's option schema to enforce strict property validation

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

✨ Finishing touches
  • 📝 Generate docstrings

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Nitpick comments (1)
code/lib/eslint-plugin/src/rules/no-uninstalled-addons.ts (1)

48-64: Schema tightening with additionalProperties: false matches the rule’s intent; consider tests/docs + version check

This change does exactly what the PR describes: it ensures the options object for no-uninstalled-addons only accepts packageJsonLocation and ignore, and will now surface invalid or misspelled keys as config errors instead of silently ignoring them. That’s a good defensive hardening and is consistent with the existing defaultOptions shape.

Two follow‑ups to consider:

  • Add/extend rule tests to cover:
    • A config using a misspelled key (e.g. packgeJsonLocation) and asserting ESLint reports an invalid option.
    • A config with an extra unknown key to ensure it is rejected.
  • Update the rule docs to explicitly state that only packageJsonLocation and ignore are allowed keys and that unknown keys will be rejected, so users understand the stricter behavior and any resulting configuration failures.

Also, please double‑check that this usage of additionalProperties: false is compatible with the minimum ESLint version that Storybook’s eslint‑plugin targets (older ESLint versions already supported it, but it’s worth confirming against the repo’s supported matrix).

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 4033f33 and 7f4a3f4.

📒 Files selected for processing (1)
  • code/lib/eslint-plugin/src/rules/no-uninstalled-addons.ts (1 hunks)
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{js,jsx,json,html,ts,tsx,mjs}

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

Use ESLint and Prettier configurations that are enforced in the codebase

Files:

  • code/lib/eslint-plugin/src/rules/no-uninstalled-addons.ts
**/*.{ts,tsx}

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

Enable TypeScript strict mode

Files:

  • code/lib/eslint-plugin/src/rules/no-uninstalled-addons.ts
code/**/*.{ts,tsx,js,jsx,mjs}

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

code/**/*.{ts,tsx,js,jsx,mjs}: Use server-side logger from 'storybook/internal/node-logger' for Node.js code
Use client-side logger from 'storybook/internal/client-logger' for browser code
Do not use console.log, console.warn, or console.error directly unless in isolated files where importing loggers would significantly increase bundle size

Files:

  • code/lib/eslint-plugin/src/rules/no-uninstalled-addons.ts
code/**/*.{ts,tsx,js,jsx}

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

Export functions that need to be tested from their modules

Files:

  • code/lib/eslint-plugin/src/rules/no-uninstalled-addons.ts
code/**/*.{js,jsx,json,html,ts,tsx,mjs}

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

code/**/*.{js,jsx,json,html,ts,tsx,mjs}: Run Prettier with --write flag to format code before committing
Run ESLint with yarn lint:js:cmd to check for linting issues and fix errors before committing

Files:

  • code/lib/eslint-plugin/src/rules/no-uninstalled-addons.ts
🧠 Learnings (1)
📓 Common learnings
Learnt from: Sidnioulz
Repo: storybookjs/storybook PR: 32458
File: code/core/src/components/components/Tabs/Tabs.stories.tsx:222-227
Timestamp: 2025-11-05T09:36:55.944Z
Learning: Repo: storybookjs/storybook PR: 32458 — In code/core/src/components/components/Button/Button.tsx (React/TypeScript), ButtonProps includes ariaLabel?: string | false and the component maps it to the DOM aria-label. Convention: ariaLabel is mandatory on all Button usages — provide a descriptive string for icon-only buttons; set ariaLabel=false when the button’s children already serve as the accessible name. Do not suggest using a raw aria-label prop on Button call sites.
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: normal

@github-actions github-actions bot removed the Stale label Dec 19, 2025
@github-actions github-actions bot added the Stale label Dec 30, 2025
@valentinpalkovic valentinpalkovic changed the title fix: disallow extra properties in eslint plugin rule options ESLint-plugin: Disallow extra properties in eslint plugin rule options Mar 10, 2026
@valentinpalkovic valentinpalkovic merged commit 9a85d2c into storybookjs:next Mar 10, 2026
54 checks passed
@github-actions github-actions bot mentioned this pull request Mar 10, 2026
10 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

3 participants