-
-
Notifications
You must be signed in to change notification settings - Fork 55
fix: share same plugin reference for default export #344
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Coverage summary from CodacySee diff coverage on Codacy
Coverage variation details
Coverage variation is the difference between the coverage for the head and common ancestor commits of the pull request branch: Diff coverage details
Diff coverage is the percentage of lines that are covered by tests out of the coverable lines that the pull request added or modified: See your quality gate settings Change summary preferences |
🦋 Changeset detectedLatest commit: 8925d8b The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
WalkthroughThe changes introduce a new changeset file documenting a patch update for the Changes
Sequence Diagram(s)sequenceDiagram
participant Importer as Importer
participant Plugin as plugin object
participant FlatConfigs as flatConfigs
Importer->>src/index.ts: import default
src/index.ts->>Plugin: create plugin object with meta, configs, rules, etc.
src/index.ts->>FlatConfigs: reference flatConfigs
src/index.ts->>Importer: return Object.assign(plugin, { flatConfigs })
Suggested labels
Suggested reviewers
Poem
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
src/index.tsOops! Something went wrong! :( ESLint: 9.27.0 Error [ERR_MODULE_NOT_FOUND]: Cannot find module '/lib/index.js' imported from /eslint.config.js 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (3)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (2)
⏰ Context from checks skipped due to timeout of 90000ms (190)
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
WalkthroughThis pull request modifies the export structure of the plugin to share the same plugin reference for default export. This change is intended to avoid the error Changes
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR refactors the default export to reuse a single plugin
object reference—preventing repeated plugin definitions that cause the Cannot redefine plugin "import-x"
error.
- Added
configs
,cjsRequire
,importXResolverCompat
, andcreateNodeResolver
into the baseplugin
object - Changed the default export to
Object.assign(plugin, { flatConfigs })
so it mergesflatConfigs
into that sameplugin
reference
This pull request is automatically built and testable in CodeSandbox. To see build info of the built libraries, click here or the icon next to each commit SHA. |
commit: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Important
Looks good to me! 👍
Reviewed everything up to 7edc5e1 in 53 seconds. Click for details.
- Reviewed
33
lines of code in1
files - Skipped
0
files when reviewing. - Skipped posting
2
draft comments. View those below. - Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. src/index.ts:151
- Draft comment:
Including 'configs', 'cjsRequire', 'importXResolverCompat', and 'createNodeResolver' within the plugin object creates a consistent shared reference. This helps avoid plugin redefinition errors. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%
<= threshold50%
This comment is purely informative and does not provide any actionable feedback or suggestions for improvement. It simply states a positive aspect of the code without prompting any further action or consideration from the PR author.
2. src/index.ts:186
- Draft comment:
Using Object.assign(plugin, { flatConfigs }) for the default export ensures the same plugin reference is used. Note that this mutates the plugin object—if immutability is a concern, consider alternatives. - Reason this comment was not posted:
Comment looked like it was already resolved.
Workflow ID: wflow_l9kKsxQDtPn8cNxz
You can customize by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.
Coverage summary from CodacySee diff coverage on Codacy
Coverage variation details
Coverage variation is the difference between the coverage for the head and common ancestor commits of the pull request branch: Diff coverage details
Diff coverage is the percentage of lines that are covered by tests out of the coverable lines that the pull request added or modified: See your quality gate settings Change summary preferences |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #344 +/- ##
=======================================
Coverage 96.25% 96.25%
=======================================
Files 92 92
Lines 4833 4833
Branches 1823 1822 -1
=======================================
Hits 4652 4652
Misses 180 180
Partials 1 1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Important
Looks good to me! 👍
Reviewed 8925d8b in 1 minute and 36 seconds. Click for details.
- Reviewed
74
lines of code in3
files - Skipped
0
files when reviewing. - Skipped posting
3
draft comments. View those below. - Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.
1. .changeset/nasty-dolls-check.md:1
- Draft comment:
The changeset note clearly states the patch and fix. Consider adding a brief rationale if needed. - Reason this comment was not posted:
Confidence changes required:0%
<= threshold50%
None
2. README.md:101
- Draft comment:
Updated import example uses named export ({ importX }) which aligns with the new plugin export pattern. - Reason this comment was not posted:
Confidence changes required:0%
<= threshold50%
None
3. src/index.ts:151
- Draft comment:
Merging additional properties into the shared plugin object and exporting it via Object.assign ensures a single plugin reference, avoiding redefinition errors. - Reason this comment was not posted:
Confidence changes required:0%
<= threshold50%
None
Workflow ID: wflow_rp3xHhHlCHkOV5BG
You can customize by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.
@@ -114,7 +114,7 @@ npm install eslint-import-resolver-typescript --save-dev | |||
|
|||
```js | |||
import js from '@eslint/js' | |||
import * as importX from 'eslint-plugin-import-x' | |||
import { importX } from 'eslint-plugin-import-x' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
does this now work with the default import too?
i.e. import importX from 'eslint-plugin-import-x'
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes and no: #285, and also no-named-as-default
.
Avoid
Cannot redefine plugin "import-x"
errorImportant
Fixes plugin redefinition error by ensuring a single reference for default export and updates import syntax in README.
Cannot redefine plugin "import-x"
error by ensuring a single plugin reference for default export insrc/index.ts
.README.md
to use{ importX }
instead of* as importX
.plugin
object withflatConfigs
usingObject.assign()
insrc/index.ts
..changeset/nasty-dolls-check.md
to document the patch update.This description was created by
for 8925d8b. You can customize this summary. It will automatically update as commits are pushed.
Summary by CodeRabbit