Skip to content

Conversation

@JounQin
Copy link
Member

@JounQin JounQin commented Apr 23, 2025

close #89

cc @voxpelli


Important

Expanded README.md to include project details, bug fixes, synchronization notes, and updated references.

  • Documentation:
    • Expanded README.md to clarify project origins, usage, and compatibility goals.
    • Added detailed list of recent bug fixes and supported features.
    • Included notes on synchronization with upstream projects and bug fix submission process.
    • Updated external references and links for related tools and projects.

This description was created by Ellipsis for d196fac. You can customize this summary. It will automatically update as commits are pushed.


Summary by CodeRabbit

  • Documentation
    • Expanded the README to clarify the project's origins, usage, and compatibility goals.
    • Added a detailed list of recent bug fixes and supported features.
    • Included notes on ongoing synchronization with upstream projects and the process for submitting bug fixes.
    • Updated external references and links for related tools and projects.

@JounQin JounQin added the documentation Improvements or additions to documentation label Apr 23, 2025
@JounQin JounQin requested a review from Copilot April 23, 2025 15:07
@coderabbitai
Copy link

coderabbitai bot commented Apr 23, 2025

Walkthrough

The README file was updated to provide a more comprehensive note about the project's origins, usage, and differences from its upstream sources. The new content clarifies that the project is a fork of both oxc-resolver and rspack-resolver, details its integration with specific ESLint plugins, and explicitly states that full compatibility with enhanced-resolve is not a goal. It also lists added features, bug fixes, and ongoing synchronization practices with upstream projects. Additional external references were included to support the new explanations.

Changes

File(s) Change Summary
README.md Expanded and clarified the note section to detail project origins, usage, feature set, bug fixes, and references.

Assessment against linked issues

Objective Addressed Explanation
Clarify README explanation regarding rspack-resolver and explain differences from upstream (#89)

Poem

A README renewed with clarity bright,
Forked from two paths, now shining with light.
Bugs are squashed, features aligned,
With upstream in mind, our code is refined.
🐇 A hop and a skip, our story’s retold—
In markdown and links, the details unfold!


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
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

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)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR.
  • @coderabbitai generate sequence diagram to generate a sequence diagram of the changes in this PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link

Copilot AI left a 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 updates the README documentation to provide more comprehensive details about the fork's features, bug fixes, and synchronization with upstream repositories.

  • Expanded description of the fork and its additional features
  • Added bullet points detailing bug fixes and improvements
  • Updated references and links to related repositories and issues
Comments suppressed due to low confidence (1)

README.md:22

  • Typo found: 'planed' should be corrected to 'planned' to accurately reflect the status of the rspack-resolver reference.
- rspack-resolver(planed): [#59](https://github.com/unrs/unrs-resolver/issues/59)

@JounQin JounQin changed the title docs: more details about the changes in this fork docs: add more details about the changes in this fork Apr 23, 2025
Copy link

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

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

Caution

Changes requested ❌

Reviewed everything up to d15a043 in 1 minute and 56 seconds. Click for details.
  • Reviewed 48 lines of code in 1 files
  • Skipped 0 files when reviewing.
  • Skipped posting 7 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. README.md:3
  • Draft comment:
    Consider rewording for clarity and explicitly differentiating from rspack-resolver. For example, replace "cause 100% compatible with [enhanced-resolve] is the non-goal of [oxc-resolver] itself" with a clearer explanation that this fork prioritizes bug fixes and additional features (e.g., pnp support, reference priority, etc.) that are not present in rspack-resolver.
  • Reason this comment was not posted:
    Comment looked like it was already resolved.
2. README.md:22
  • Draft comment:
    Typo: Replace "planed" with "planned".
  • Reason this comment was not posted:
    Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 20% vs. threshold = 50% While this is a legitimate spelling error, we need to consider whether spelling fixes in documentation are worth commenting on. The rules don't explicitly address spelling corrections, but they do emphasize only making comments that require clear code changes. Documentation typos are generally minor issues that don't affect functionality. The spelling error could potentially confuse readers or appear unprofessional. Documentation quality does matter for project maintainability. However, this is a very minor typo that doesn't impact understanding - "planed" vs "planned" is obvious from context. Making comments about such minor documentation issues creates noise in the review process. While technically correct, this comment about a minor documentation spelling error doesn't meet the bar for being important enough to include in the review.
3. README.md:3
  • Draft comment:
    Clarify the wording. Consider rephrasing to: "Since achieving 100% compatibility with [enhanced-resolve] is not our goal, we add enhancements such as pnp support..." for better readability.
  • Reason this comment was not posted:
    Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 10% vs. threshold = 50% While the suggested rewording does improve readability and grammar, the rules state not to make purely informative comments or comments that don't require clear code changes. Documentation style suggestions are generally not actionable enough to warrant a comment. The meaning is still clear in the original. The text does have grammatical issues that could confuse readers. Clear documentation is important for project success. While clear docs are valuable, the bar for README comments should be high - the meaning is still understandable and this is a style preference rather than a critical issue. Delete this comment as it's a purely stylistic documentation suggestion that doesn't highlight a critical issue requiring change.
4. README.md:4
  • Draft comment:
    Consider adding a dedicated paragraph that explicitly outlines how this fork differs from [rspack-resolver] (e.g. listing additional bug fixes and features) to better address issue #89.
  • Reason this comment was not posted:
    Comment did not seem useful. Confidence is useful = 0% <= threshold 50% This comment is asking the PR author to update the PR description by adding a paragraph about how the fork differs from another project. This violates the rule against asking the author to update the PR description.
5. README.md:3
  • Draft comment:
    Consider rephrasing the sentence on line 3 for clarity. For example, instead of "cause 100% compatible with [enhanced-resolve]...", you might say "because 100% compatibility with [enhanced-resolve] is not the goal." This will improve readability.
  • Reason this comment was not posted:
    Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 10% vs. threshold = 50% The rules state not to make purely informative comments. While this is a valid grammar improvement, it's a minor stylistic change that doesn't affect functionality. The meaning is still clear in the original. Documentation style suggestions like this are not critical enough to warrant a comment. The grammar could be confusing to non-native English speakers. Poor documentation can impact project usability. While documentation quality matters, this minor grammar issue doesn't significantly impact understanding. The rules explicitly say not to make purely informative comments. Delete this comment as it's a purely stylistic suggestion that doesn't affect functionality or significantly impact understanding.
6. README.md:13
  • Draft comment:
    On line 13, consider changing 'Raspberry PI' to 'Raspberry Pi' to match the common naming convention.
  • Reason this comment was not posted:
    Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 0% vs. threshold = 50% While technically correct that "Raspberry Pi" is the official product name, this is a very minor stylistic issue in a README file. The meaning is completely clear either way. The comment doesn't point out any actual code or functionality issues. According to the rules, we should not make purely informative comments or comments about obvious/unimportant things. The official branding guidelines for Raspberry Pi do specify the correct capitalization. Documentation accuracy could be considered important. While technically accurate, this level of nitpicking about capitalization in a README bullet point doesn't meet the bar of being an important enough issue to flag in a PR review. Delete this comment as it's too minor and doesn't affect functionality.
7. README.md:22
  • Draft comment:
    On line 22, please change '(planed)' to '(planned)' for correct spelling.
  • Reason this comment was not posted:
    Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 10% vs. threshold = 50% While this is a legitimate spelling error in newly added text, spelling corrections in documentation are generally considered minor and not worth commenting on in PRs. They can be fixed directly or in a follow-up PR. The error doesn't impact code functionality or understanding. The spelling error could potentially confuse non-native English speakers. Documentation quality does matter for project professionalism. While documentation quality matters, this is an extremely minor spelling issue that doesn't significantly impact readability or understanding. The meaning is still clear from context. Delete this comment as it's too minor of an issue to warrant a PR comment. Spelling fixes can be handled separately.

Workflow ID: wflow_dHYxbFAEYIAYlBrN

You can customize Ellipsis by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.

Copy link

@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: 1

🧹 Nitpick comments (5)
README.md (5)

5-15: Ensure consistent style and parallelism in the bullet list

The bug-fix list mixes verb forms and noun phrases, which can hamper readability. Consider starting each item with a verb in the same tense and placing references at the end, for example:

- - takes `paths` and `references` into account [at the same time](https://github.com/unrs/unrs-resolver/pull/12)
+ - Handle `paths` and `references` simultaneously [#12]

Apply similar adjustments to maintain a uniform style across all list entries.

🧰 Tools
🪛 LanguageTool

[uncategorized] ~5-~5: This verb may not be in the correct tense. Consider changing the tense to fit the context better.
Context: ...ra-dev/rspack/issues/2236). > > We also fix several bugs reported by [eslint-plugin...

(AI_EN_LECTOR_REPLACEMENT_VERB_TENSE)


17-17: Rephrase informal list placeholder

The phrase “The list could be longer in the future, but we don't want to make it too long here.” is informal. Consider a more concise alternative:

- > The list could be longer in the future, but we don't want to make it too long here.
+ > This list may be extended as needed in future updates.

19-23: Correct typo and improve spacing in sync section

  • Fix the typo “(planed)” → “(planned)”.
  • Add a space before the parenthesis for readability.
- - `rspack-resolver`(planed): [#59](https://github.com/unrs/unrs-resolver/issues/59)
+ - `rspack-resolver` (planned): [#59](https://github.com/unrs/unrs-resolver/issues/59)
🧰 Tools
🪛 LanguageTool

[uncategorized] ~21-~21: Loose punctuation mark.
Context: ...the latest changes. > > - oxc-resolver: [#15](https://github.com/unrs/unrs-reso...

(UNLIKELY_OPENING_PUNCTUATION)


24-27: Replace “Last but not least” with a concise transition

“Last but not least” can be verbose—consider “Finally,” for a cleaner style. For example:

- > Last but not least, we prepare some bug fix PRs first on our side and PR back into upstream projects, and we will keep doing this in the future:
+ > Finally, we prepare bug fix PRs on our side before submitting upstream and will continue this practice:
🧰 Tools
🪛 LanguageTool

[style] ~24-~24: ‘Last but not least’ might be wordy. Consider a shorter alternative.
Context: ...b.com//issues/59) > > Last but not least, we prepare some bug fix PRs first on o...

(EN_WORDINESS_PREMIUM_LAST_BUT_NOT_LEAST)


[uncategorized] ~26-~26: Loose punctuation mark.
Context: ...this in the future: > > - oxc-resolver: [#84](https://github.com/unrs/unrs-reso...

(UNLIKELY_OPENING_PUNCTUATION)


284-288: Alphabetize and group new link definitions for clarity

Consider ordering the newly added reference definitions alphabetically and grouping them with the existing links to improve maintainability:

- [rspack-resolver]:                      https://github.com/web-infra-dev/rspack-resolver
- [eslint-plugin-import-x]:              https://github.com/un-ts/eslint-plugin-import-x
- [eslint-import-resolver-typescript]:   https://github.com/import-js/eslint-import-resolver-typescript
- [napi-postinstall]:                    https://github.com/un-ts/napi-postinstall
- [mimalloc-safe]:                       https://github.com/napi-rs/mimalloc-safe
+ [eslint-import-resolver-typescript]:   https://github.com/import-js/eslint-import-resolver-typescript
+ [eslint-plugin-import-x]:              https://github.com/un-ts/eslint-plugin-import-x
+ [mimalloc-safe]:                       https://github.com/napi-rs/mimalloc-safe
+ [napi-postinstall]:                    https://github.com/un-ts/napi-postinstall
+ [rspack-resolver]:                     https://github.com/web-infra-dev/rspack-resolver
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 610d9f3 and d15a043.

📒 Files selected for processing (1)
  • README.md (2 hunks)
🧰 Additional context used
🪛 LanguageTool
README.md

[uncategorized] ~5-~5: This verb may not be in the correct tense. Consider changing the tense to fit the context better.
Context: ...ra-dev/rspack/issues/2236). > > We also fix several bugs reported by [eslint-plugin...

(AI_EN_LECTOR_REPLACEMENT_VERB_TENSE)


[uncategorized] ~21-~21: Loose punctuation mark.
Context: ...the latest changes. > > - oxc-resolver: [#15](https://github.com/unrs/unrs-reso...

(UNLIKELY_OPENING_PUNCTUATION)


[style] ~24-~24: ‘Last but not least’ might be wordy. Consider a shorter alternative.
Context: ...b.com//issues/59) > > Last but not least, we prepare some bug fix PRs first on o...

(EN_WORDINESS_PREMIUM_LAST_BUT_NOT_LEAST)


[uncategorized] ~26-~26: Loose punctuation mark.
Context: ...this in the future: > > - oxc-resolver: [#84](https://github.com/unrs/unrs-reso...

(UNLIKELY_OPENING_PUNCTUATION)

⏰ Context from checks skipped due to timeout of 90000ms (2)
  • GitHub Check: Codacy Static Code Analysis
  • GitHub Check: Analyze (javascript-typescript)

Copy link

@ellipsis-dev ellipsis-dev bot left a 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 2fd9a57 in 1 minute and 55 seconds. Click for details.
  • Reviewed 48 lines of code in 1 files
  • Skipped 0 files when reviewing.
  • Skipped posting 7 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. README.md:3
  • Draft comment:
    Consider rephrasing for clarity. For example, replace "cause 100% compatible with [enhanced-resolve] is the non-goal of [oxc-resolver] itself" with clearer wording such as "since 100% compatibility with [enhanced-resolve] is not the goal of [oxc-resolver], we add...".
  • Reason this comment was not posted:
    Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 0% vs. threshold = 50% The rules state not to make purely informative comments. While the grammar could be improved, this is a minor stylistic suggestion that doesn't affect functionality. The meaning is still understandable despite the grammatical issue. The rules emphasize focusing on code changes requiring fixes rather than documentation style. The grammar error does make the documentation less professional and could confuse non-native English speakers. Clear documentation is important for open source projects. While documentation clarity is valuable, the review rules explicitly state not to make purely informative comments. The meaning is still comprehensible despite the grammar issue. This comment should be deleted as it is a purely stylistic documentation suggestion that doesn't affect functionality or require code changes.
2. README.md:22
  • Draft comment:
    Typo: change "(planed)" to "(planned)" for proper spelling.
  • Reason this comment was not posted:
    Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 10% vs. threshold = 50% While this is a legitimate spelling error in newly added text, spelling fixes in documentation are generally considered minor and not worth commenting on unless they significantly impact meaning or readability. This particular typo is quite minor and obvious. The word is definitely misspelled and fixing it would improve the quality of the documentation. Documentation quality matters. While documentation quality matters, this is an extremely minor typo that doesn't impact understanding. The PR author can easily spot and fix such obvious typos during their own review. This comment should be deleted as it points out an extremely minor documentation issue that doesn't meaningfully impact readability or understanding.
3. README.md:1
  • Draft comment:
    The diff lists bug fixes and upstream syncs, but does not clearly explain how this fork differs from rspack-resolver. Consider adding a dedicated section comparing key differences with rspack-resolver to fully resolve issue #89.
  • Reason this comment was not posted:
    Comment did not seem useful. Confidence is useful = 0% <= threshold 50% This comment is asking the PR author to update the PR description by adding a section comparing key differences with rspack-resolver. This violates the rule against asking the author to update the PR description.
4. README.md:3
  • Draft comment:
    Consider rephrasing: replace 'cause' with 'because' and clarify that this fork is tailored for ESLint integration. Explicitly mention that unlike rspack-resolver, this fork intentionally diverges from aiming for 100% enhanced-resolve compatibility by including targeted bug fixes and additional features.
  • Reason this comment was not posted:
    Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 0% vs. threshold = 50% While the suggested rewording is arguably more professional, this is just a style preference for the README. The current wording is understandable and gets the point across. The rules state not to make purely informative comments or comments that don't require clear code changes. Documentation style suggestions are not critical changes. The writing could be more professional, and clarity in documentation is important for users understanding the project's purpose. However, the current wording is functional and clear enough. This is a style preference rather than a necessary fix. Delete this comment as it suggests only stylistic changes to documentation without addressing any technical issues or required code changes.
5. README.md:22
  • Draft comment:
    Typo: 'planed' should be 'planned'.
  • Reason this comment was not posted:
    Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 0% vs. threshold = 50% While this is a legitimate spelling error, I need to consider whether fixing typos in documentation is valuable enough to warrant a comment. The error is minor and doesn't impact understanding. However, since this is in the main README file which is the public face of the project, maintaining professional quality may be important. Documentation quality matters for project professionalism. A misspelling in the main README could reflect poorly on the project. While documentation quality matters, this is an extremely minor typo that doesn't impact understanding. The rules state we should not make purely informative comments. Following the rules strictly, we should not keep comments that are purely informative without requiring code changes. This spelling fix, while accurate, falls into that category.
6. README.md:13
  • Draft comment:
    Typo: 'Raspberry PI' should be 'Raspberry Pi'.
  • Reason this comment was not posted:
    Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 0% vs. threshold = 50% While the comment is technically correct about the proper capitalization of "Raspberry Pi", this is a very minor issue in documentation. The rules state not to make purely informative comments or comments about obvious/unimportant things. A capitalization fix in documentation is not a critical code change that needs to be flagged. The proper branding of "Raspberry Pi" could be considered important for professional documentation. Some might argue that maintaining correct product names is valuable. While proper branding is good, this is a minor documentation issue that doesn't impact functionality. The rules clearly state to avoid purely informative comments and unimportant issues. This comment should be deleted as it addresses a minor documentation issue and doesn't suggest any meaningful code changes.
7. README.md:22
  • Draft comment:
    Typo: 'planed' should be corrected to 'planned'.
  • Reason this comment was not posted:
    Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 0% vs. threshold = 50% While this is technically correct - "planed" is misspelled and should be "planned" - the review rules state we should not make purely informative comments. This is a minor typo in documentation that doesn't affect functionality. The rules emphasize focusing on code logic issues rather than documentation fixes. The typo could potentially confuse readers about the project's roadmap timing. Documentation clarity is important for project maintainability. While documentation clarity matters, this is an extremely minor typo that most readers would understand correctly from context. The review rules explicitly discourage purely informative comments. This comment should be deleted as it is a minor documentation issue that doesn't affect functionality and violates the rule against purely informative comments.

Workflow ID: wflow_tzeU4cm9dX7nyxY8

You can customize Ellipsis by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.

Copy link

@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

♻️ Duplicate comments (2)
README.md (2)

1-3: 🛠️ Refactor suggestion

Replace unsupported [!NOTE] syntax with standard Markdown note
GitHub Flavored Markdown doesn’t support [!NOTE], so the admonition won’t render as intended. Please replace it with a standard bolded note.

- > [!NOTE]
+ > **Note:**

22-22: 🛠️ Refactor suggestion

Fix typo in ‘planned’
The word “planed” is a typo and should be “planned”. Also add a space for readability.

- > - `rspack-resolver`(planed): [#59](https://github.com/unrs/unrs-resolver/issues/59)
+ > - `rspack-resolver` (planned): [#59](https://github.com/unrs/unrs-resolver/issues/59)
🧹 Nitpick comments (3)
README.md (3)

3-3: Rephrase and clarify the introductory sentence
The sentence is lengthy and uses “cause” informally. Consider splitting and restructuring for better readability:

- > This is a fork of [oxc-resolver] and [rspack-resolver], and will be used in [eslint-plugin-import-x] and [eslint-import-resolver-typescript] cause 100% compatible with [enhanced-resolve] is the non-goal of [oxc-resolver] itself, we add [enhanced-resolve] specific features like [`pnp support`](https://github.com/web-infra-dev/rspack/issues/2236).
+ > This project is a fork of [oxc-resolver] and [rspack-resolver], used by [eslint-plugin-import-x] and [eslint-import-resolver-typescript]. Full compatibility with [enhanced-resolve] is not the goal; instead, we’ve added features such as [`pnp support`](https://github.com/web-infra-dev/rspack/issues/2236).

5-5: Use past tense for consistency
Change “We also fix several bugs reported by …” to the past tense for consistency with the rest of the changelog-style list.

- > We also fix several bugs reported by [eslint-plugin-import-x] and [eslint-import-resolver-typescript] users:
+ > We also fixed several bugs reported by [eslint-plugin-import-x] and [eslint-import-resolver-typescript] users:
🧰 Tools
🪛 LanguageTool

[uncategorized] ~5-~5: This verb may not be in the correct tense. Consider changing the tense to fit the context better.
Context: ...ra-dev/rspack/issues/2236). > > We also fix several bugs reported by [eslint-plugin...

(AI_EN_LECTOR_REPLACEMENT_VERB_TENSE)


24-24: Consider a more concise transition phrase
“Last but not least,” is wordy. You could shorten it to “Finally,” to improve flow.

- > Last but not least, we prepare some bug fix PRs first on our side and PR back into upstream projects, and we will keep doing this in the future:
+ > Finally, we prepare bug fix PRs on our side and submit them upstream; we’ll continue this practice in the future:
🧰 Tools
🪛 LanguageTool

[style] ~24-~24: ‘Last but not least’ might be wordy. Consider a shorter alternative.
Context: ...b.com//issues/59) > > Last but not least, we prepare some bug fix PRs first on o...

(EN_WORDINESS_PREMIUM_LAST_BUT_NOT_LEAST)

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between d15a043 and 2fd9a57.

📒 Files selected for processing (1)
  • README.md (2 hunks)
🧰 Additional context used
🪛 LanguageTool
README.md

[uncategorized] ~5-~5: This verb may not be in the correct tense. Consider changing the tense to fit the context better.
Context: ...ra-dev/rspack/issues/2236). > > We also fix several bugs reported by [eslint-plugin...

(AI_EN_LECTOR_REPLACEMENT_VERB_TENSE)


[uncategorized] ~21-~21: Loose punctuation mark.
Context: ...the latest changes: > > - oxc-resolver: [#15](https://github.com/unrs/unrs-reso...

(UNLIKELY_OPENING_PUNCTUATION)


[style] ~24-~24: ‘Last but not least’ might be wordy. Consider a shorter alternative.
Context: ...b.com//issues/59) > > Last but not least, we prepare some bug fix PRs first on o...

(EN_WORDINESS_PREMIUM_LAST_BUT_NOT_LEAST)


[uncategorized] ~26-~26: Loose punctuation mark.
Context: ...this in the future: > > - oxc-resolver: [#84](https://github.com/unrs/unrs-reso...

(UNLIKELY_OPENING_PUNCTUATION)

⏰ Context from checks skipped due to timeout of 90000ms (1)
  • GitHub Check: Codacy Static Code Analysis

Copy link

@ellipsis-dev ellipsis-dev bot left a 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 d196fac in 1 minute and 44 seconds. Click for details.
  • Reviewed 48 lines of code in 1 files
  • Skipped 0 files when reviewing.
  • Skipped posting 5 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. README.md:3
  • Draft comment:
    Consider rewording the introductory sentence for clarity. For example, replace “cause 100% compatible with” with “since 100% compatibility with” to improve readability.
  • Reason this comment was not posted:
    Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 0% vs. threshold = 50% While the suggestion would improve the text, our rules state "Do NOT make purely informative comments" and "Do NOT comment unless there is clearly a code change required." This is a style/grammar suggestion that doesn't affect functionality. The meaning is still clear despite the minor grammatical issue. The text is technically incorrect grammatically, and improving documentation clarity could be considered important for project maintainability. However, the rules are clear that we should only keep comments that require actual code changes. Editorial suggestions about documentation wording should be handled through other channels. Delete this comment as it's purely editorial feedback about documentation wording rather than a required code change.
2. README.md:1
  • Draft comment:
    The note doesn’t explicitly explain how this fork differs from rspack-resolver. It may help to add a brief section clarifying the key differences, e.g., the additional bug fixes and enhanced feature support.
  • Reason this comment was not posted:
    Comment did not seem useful. Confidence is useful = 0% <= threshold 50% This comment is asking the PR author to update the PR description or add additional information to clarify differences, which violates the rule against asking for updates to the PR description.
3. README.md:3
  • Draft comment:
    Please clarify how this fork differs from rspack-resolver. The issue requires an explanation that, unlike rspack-resolver (which aims for 100% enhanced-resolve compatibility), this fork is tailored for ESLint plugin integration and includes specific bug fixes. Also, replace the informal 'cause' with 'because' and consider hyphenating 'enhanced-resolve-specific features' for clarity.
  • Reason this comment was not posted:
    Comment did not seem useful. Confidence is useful = 50% <= threshold 50% The comment is asking for clarification on how a fork differs from another project, which is not allowed as it asks the author to explain. However, it also provides a suggestion to replace 'cause' with 'because' and to consider hyphenating a phrase for clarity, which are valid suggestions. The comment is partially useful.
4. README.md:3
  • Draft comment:
    The sentence on line 3 is a bit confusing because it uses 'cause' instead of a clearer word like 'because'. Consider rephrasing it for better clarity, e.g., "... and will be used in [eslint-plugin-import-x] and [eslint-import-resolver-typescript] because being 100% compatible with [enhanced-resolve] is not the goal of [oxc-resolver] itself. Instead, we add [enhanced-resolve]-specific features like pnp support ..."
  • Reason this comment was not posted:
    Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 10% vs. threshold = 50% While the suggestion would make the text slightly more formal, this is a minor stylistic change. The current wording is understandable and informal language in documentation is acceptable. The rules state not to make purely informative comments or comments that don't require clear code changes. The text could be slightly clearer with the suggested change. Poor documentation can impact project usability. While documentation clarity matters, this change is too minor to warrant a comment. The meaning is clear enough as is. Delete this comment as it's a minor stylistic suggestion that doesn't materially improve understanding.
5. README.md:13
  • Draft comment:
    On line 13, 'Raspberry PI 4 aarch64' should likely be 'Raspberry Pi 4 aarch64' to match the proper branding of the Raspberry Pi. Consider correcting the capitalization of 'PI' to 'Pi'.
  • Reason this comment was not posted:
    Decided after close inspection that this draft comment was likely wrong and/or not actionable: usefulness confidence = 0% vs. threshold = 50% The rules state not to make purely informative comments and to focus on logic rather than styling. While technically correct about the branding, this is a minor stylistic issue in documentation that doesn't affect functionality. The meaning is clear regardless of capitalization. The branding could matter for professional appearance and searchability. Incorrect branding might make the documentation look less polished. While proper branding is nice to have, our rules explicitly state to focus on logic and avoid purely informative comments. This is exactly the kind of minor, non-functional nitpick we should avoid. Delete this comment as it's purely about documentation style and doesn't affect functionality or code quality.

Workflow ID: wflow_ORJ3QFgF8anQ4nml

You can customize Ellipsis by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.

Copy link

@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 (4)
README.md (4)

1-3: Improve clarity and grammar in the note introduction.

The phrase
“and will be used in [eslint-plugin-import-x] and [eslint-import-resolver-typescript] cause 100% compatible with [enhanced-resolve] is the non-goal of [oxc-resolver] itself”
is awkward and hard to parse. Consider rewriting for readability, for example:

> [!NOTE]
> 
- This is a fork of [oxc-resolver] and [rspack-resolver], and will be used in [eslint-plugin-import-x] and [eslint-import-resolver-typescript] cause 100% compatible with [enhanced-resolve] is the non-goal of [oxc-resolver] itself, we add [enhanced-resolve] specific features like [`pnp support`](...).
+ **Note:**
+ This project is a fork of [oxc-resolver] and [rspack-resolver], used by [eslint-plugin-import-x] and [eslint-import-resolver-typescript]. Full compatibility with [enhanced-resolve] is not a goal of [oxc-resolver]; therefore, we’ve added specific enhancements like [`pnp support`](https://github.com/web-infra-dev/rspack/issues/2236).

5-5: Use consistent tense for bug fixes.

“We also fix several bugs reported by …” reads as present simple. For consistency with past work, consider “We have also fixed several bugs reported by …”.

🧰 Tools
🪛 LanguageTool

[uncategorized] ~5-~5: This verb may not be in the correct tense. Consider changing the tense to fit the context better.
Context: ...ra-dev/rspack/issues/2236). > > We also fix several bugs reported by [eslint-plugin...

(AI_EN_LECTOR_REPLACEMENT_VERB_TENSE)


22-22: Add a space before the parenthesis.

The entry

- `rspack-resolver`(planned): [#59](…)

should be

- `rspack-resolver`(planned): [#59](…)
+ `rspack-resolver` (planned): [#59](…)

to follow Markdown link and parenthesis spacing conventions.


24-24: Consider a shorter alternative to “Last but not least.”

“Last but not least” can be wordy in documentation. You might replace it with “Finally,” or “Lastly,” for conciseness.

🧰 Tools
🪛 LanguageTool

[style] ~24-~24: ‘Last but not least’ might be wordy. Consider a shorter alternative.
Context: ...b.com//issues/59) > > Last but not least, we prepare some bug fix PRs first on o...

(EN_WORDINESS_PREMIUM_LAST_BUT_NOT_LEAST)

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 2fd9a57 and d196fac.

📒 Files selected for processing (1)
  • README.md (2 hunks)
🧰 Additional context used
🧠 Learnings (1)
README.md (1)
Learnt from: JounQin
PR: unrs/unrs-resolver#92
File: README.md:2-3
Timestamp: 2025-04-23T15:13:13.885Z
Learning: GitHub Flavored Markdown supports the admonition syntax using `> [!NOTE]`, `> [!WARNING]`, `> [!IMPORTANT]` and other similar variants as documented in https://github.com/orgs/community/discussions/16925.
🪛 LanguageTool
README.md

[uncategorized] ~5-~5: This verb may not be in the correct tense. Consider changing the tense to fit the context better.
Context: ...ra-dev/rspack/issues/2236). > > We also fix several bugs reported by [eslint-plugin...

(AI_EN_LECTOR_REPLACEMENT_VERB_TENSE)


[uncategorized] ~21-~21: Loose punctuation mark.
Context: ...the latest changes: > > - oxc-resolver: [#15](https://github.com/unrs/unrs-reso...

(UNLIKELY_OPENING_PUNCTUATION)


[style] ~24-~24: ‘Last but not least’ might be wordy. Consider a shorter alternative.
Context: ...b.com//issues/59) > > Last but not least, we prepare some bug fix PRs first on o...

(EN_WORDINESS_PREMIUM_LAST_BUT_NOT_LEAST)


[uncategorized] ~26-~26: Loose punctuation mark.
Context: ...this in the future: > > - oxc-resolver: [#84](https://github.com/unrs/unrs-reso...

(UNLIKELY_OPENING_PUNCTUATION)

⏰ Context from checks skipped due to timeout of 90000ms (1)
  • GitHub Check: Codacy Static Code Analysis
🔇 Additional comments (1)
README.md (1)

284-288: Link definitions for newly introduced references are present.

All new reference links—[rspack-resolver], [eslint-plugin-import-x], [eslint-import-resolver-typescript], [napi-postinstall], and [mimalloc-safe]—have corresponding definitions and valid URLs.

@JounQin JounQin closed this Apr 23, 2025
@JounQin JounQin reopened this Apr 23, 2025
@sonarqubecloud
Copy link

@JounQin JounQin merged commit cb6643b into main Apr 23, 2025
9 checks passed
@JounQin JounQin deleted the docs/fork branch April 23, 2025 15:34
Copy link

@voxpelli voxpelli left a comment

Choose a reason for hiding this comment

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

A clear improvement 👏

To be fair though I still don’t truly understand what sets this project apart from rspack-resolver, apart from possibly being a staging area for bug fixes that are yet to make it into rspack-resolver and as such serve to deliver those fixes quicker to the users of this module?

Suggestion:

Add a list / table named Which should I pick? that shows your perception of when a developer should pick this vs one of the other two.

Is this module eg just meant for the two ESLint related modules or is it meant as a more general replacement for the other two?

Reviewed and written on my phone while walking home from work, quality of review suffers from it

> [!NOTE]
> This is a fork of [oxc-resolver] and [rspack-resolver](https://github.com/web-infra-dev/rspack-resolver), cause 100% compatible with [enhanced-resolve] is the non-goal of [oxc-resolver] itself, we may add [enhanced-resolve] specific features like [`pnp support`](https://github.com/web-infra-dev/rspack/issues/2236) and [`alternative support`](https://github.com/web-infra-dev/rspack/issues/5052) in the future.
>
> This is a fork of [oxc-resolver] and [rspack-resolver], and will be used in [eslint-plugin-import-x] and [eslint-import-resolver-typescript] cause 100% compatible with [enhanced-resolve] is the non-goal of [oxc-resolver] itself, we add [enhanced-resolve] specific features like [`pnp support`](https://github.com/web-infra-dev/rspack/issues/2236).

Choose a reason for hiding this comment

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

Suggestion: Split into two sentences. One concerned with forking from the projects and another for compatibility with enhanced-resolve, the latter mentioning that rspack-resolver has that same goal.

Maybe something like:

We align with [rspack-resolver] on being compatible with [enhanced-resolve] – a non-goal for [oxc-resolver].

Copy link
Member Author

@JounQin JounQin Apr 23, 2025

Choose a reason for hiding this comment

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

@voxpelli Would you like to raise a PR instead? Actually I'm not a native English speaker, so there must be some words/sentences could be improved.

@JounQin
Copy link
Member Author

JounQin commented Apr 23, 2025

To be fair though I still don’t truly understand what sets this project apart from rspack-resolver, apart from possibly being a staging area for bug fixes that are yet to make it into rspack-resolver and as such serve to deliver those fixes quicker to the users of this module?

The @rspack/resolver doesn't sync with oxc-resolver at all, so bug fixes in oxc-loader could be missing in @rspack/resolver as I mentioned that web-infra-dev/rspack-resolver#54 should be replaced by oxc-project/oxc-resolver#443 actually, for example. I was not so familiar with Rust and the whole code base at that time.

Add a list / table named Which should I pick? that shows your perception of when a developer should pick this vs one of the other two.

IMO all users should prefer using this downstream project based on the both upstreams, all upstream bugs will be resolved, all features will be included in this fork as possible, no need to mention the legacy npm issue here.

And not all fixes/features are possible to be backported into both upstreams because they're not in upstream/downstream relationship now.

And I proposed the upstreams should be aligned in the infrastructure, see also oxc-project/oxc-resolver#466

But it's impossible asking migration for rolldown-vite using oxc-resolver and rspack using @rspack/resolver, of course.

Is this module eg just meant for the two ESLint related modules or is it meant as a more general replacement for the other two?

It was made for those two modules at the first place, at that time, it's named rspack-resolver, and then I fixed a lot of issues which can be applied in other projects, so I rebranded it as unrs-resolver now.

Reviewed and written on my phone while walking home from work, quality of review suffers from it

I'm also on my phone. :)

If you think there is anything missing in the document, feel free to raise a PR to improve it based on our discussion. I'm a bit tired now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Fix README explanation in regards to rspack-resolver

3 participants