Skip to content

fix(no-export): don't report on assignment to locals named module#1976

Merged
G-Rath merged 7 commits into
jest-community:mainfrom
eryue0220:fix/no-export-rule-update
Jun 30, 2026
Merged

fix(no-export): don't report on assignment to locals named module#1976
G-Rath merged 7 commits into
jest-community:mainfrom
eryue0220:fix/no-export-rule-update

Conversation

@eryue0220

@eryue0220 eryue0220 commented Jun 23, 2026

Copy link
Copy Markdown
Contributor

Fixes false positives in jest/no-export when detecting CommonJS export assignments in test files.

  1. Require the exact exports property name instead of matching export typos via /^exports?$/.
  2. Respect variable shadowing: ignore assignments when module or exports refers to a local binding.

Follow-up PRs will cover missed detection patterns (module["exports"], nested chains) and treating describe blocks as test files.

@G-Rath G-Rath left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I'm heading to bed right now but can review tomorrow.

Could you pull some of these changes out into dedicated PRs so they can have their own changelog entries? That'll also hopefully help with getting coverage sorted

eryue0220 and others added 2 commits June 24, 2026 14:48
Require the exact exports property name instead of matching export typos,
and ignore assignments when module or exports refers to a local binding.

Co-authored-by: Cursor <cursoragent@cursor.com>
Add a test case for module[exports] assignments when exports refers to
the global CommonJS binding, satisfying the 100% coverage requirement.

Co-authored-by: Cursor <cursoragent@cursor.com>
@eryue0220

Copy link
Copy Markdown
Contributor Author

I'm heading to bed right now but can review tomorrow.

Could you pull some of these changes out into dedicated PRs so they can have their own changelog entries? That'll also hopefully help with getting coverage sorted

Hi @G-Rath,

I've refactored the pr into three, another two are #1977 and #1978.

And now you can review them when you're available.

@G-Rath G-Rath left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

if I've understood right, this is mainly about ensuring we're checking the global module and exports? if so I'd be keen to land that specifically next as it's a clear bug

@eryue0220

Copy link
Copy Markdown
Contributor Author

if I've understood right, this is mainly about ensuring we're checking the global module and exports? if so I'd be keen to land that specifically next as it's a clear bug

Yes, that's right. This PR is now scoped to the global CommonJS module / exports handling: avoiding false positives for shadowed bindings and requiring the actual exports accessor.

@G-Rath

G-Rath commented Jun 27, 2026

Copy link
Copy Markdown
Collaborator

cool in that case can you not drop support for module.export, and remove the support for module[exports] = function() {};? as they're unrelated to checking the scope of module

@eryue0220

Copy link
Copy Markdown
Contributor Author

cool in that case can you not drop support for module.export, and remove the support for module[exports] = function() {};? as they're unrelated to checking the scope of module

Okay, I've updated.

'export default function () {}',
'module.exports = function(){}',
'module.exports.myThing = "valid";',
dedent`

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

this is the main test I expect to see in your diff for "don't report on assignment to locals named module" - so long as the other tests and the related code required to support them is present, you have changes to make

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

this is the main test I expect to see in your diff for "don't report on assignment to locals named module" - so long as the other tests and the related code required to support them is present, you have changes to make

Updated.

…nt-plugin-jest into fix/no-export-rule-update

* 'fix/no-export-rule-update' of github.com:eryue0220/eslint-plugin-jest:
  chore(deps): update danger/danger-js action to v13.0.10 (jest-community#1983)
  chore(release): 29.15.3 [skip ci]
  fix(no-export): treat describe blocks as test files (jest-community#1978)
@G-Rath G-Rath changed the title fix(no-export): fix false positives and missed cases for CommonJS exports fix(no-export): don't report on assignment to locals named module Jun 30, 2026
@G-Rath G-Rath merged commit da02c0c into jest-community:main Jun 30, 2026
github-actions Bot pushed a commit that referenced this pull request Jun 30, 2026
## [29.15.4](v29.15.3...v29.15.4) (2026-06-30)

### Bug Fixes

* **no-export:** don't report on assignment to locals named `module` ([#1976](#1976)) ([da02c0c](da02c0c))
@github-actions

Copy link
Copy Markdown

🎉 This PR is included in version 29.15.4 🎉

The release is available on:

Your semantic-release bot 📦🚀

@eryue0220 eryue0220 deleted the fix/no-export-rule-update branch June 30, 2026 04:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants