-
-
Notifications
You must be signed in to change notification settings - Fork 5
chore: update all left references, add WASM example #172
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
|
Warning Rate limit exceeded@JounQin has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 21 minutes and 29 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (3)
WalkthroughThis update replaces all references to Changes
Possibly related PRs
Suggested labels
Poem
✨ 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 (
|
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 renames all references from oxc_resolver to unrs_resolver across code, docs, and config, and adds a WASM usage example link.
- Updated crate and npm package identifiers in Rust source and JS docs.
- Adjusted tracing env vars and fuzz targets to use the new crate name.
- Introduced a "Supports WASM" section in the README with an example link.
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| src/lib.rs | Updated crate/npm links to unrs_resolver |
| napi/src/tracing.rs | Replaced OXC_LOG with UNRS_LOG and updated doc comments |
| fuzz/fuzz_targets/resolver.rs | Switched oxc_resolver::Resolver to unrs_resolver::Resolver |
| fuzz/Cargo.toml | Replaced oxc_resolver dependency with unrs_resolver |
| README.md | Added WASM example section and updated all residual oxc_resolver references |
Comments suppressed due to low confidence (2)
README.md:79
- [nitpick] Consider expanding the WASM section with a minimal code snippet showing how to import and invoke the WASM bundle alongside the example link, for quicker developer onboarding.
### Supports WASM
README.md:79
- The new WASM support isn't covered by the existing test suite; consider adding a CI job or dedicated tests to validate the WASM build and example usage.
### Supports WASM
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 1b76e4d in 49 seconds. Click for details.
- Reviewed
123lines of code in5files - Skipped
0files when reviewing. - Skipped posting
1draft 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/lib.rs:1
- Draft comment:
Update the crate header: Change '# Oxc Resolver' to '# UnRS Resolver' to match the new naming in documentation and external links. - Reason this comment was not posted:
Comment was not on a location in the diff, so it can't be submitted as a review comment.
Workflow ID: wflow_CfQPEWx7ckKE045K
You can customize by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.
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 f1462fa in 39 seconds. Click for details.
- Reviewed
71lines of code in3files - Skipped
0files when reviewing. - Skipped posting
3draft 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:76
- Draft comment:
Docs update looks consistent. The new 'Supports WASM' section and debug log references are correctly updated to 'unrs_resolver'. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
2. fuzz/Cargo.toml:23
- Draft comment:
Dependency update from oxc_resolver to unrs_resolver is correct. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
3. fuzz/fuzz_targets/resolver.rs:4
- Draft comment:
Import update to 'unrs_resolver::Resolver' is correctly reflected after the package rename. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
Workflow ID: wflow_3ldBsfZdFoVi8ZE5
You can customize by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.
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.
Actionable comments posted: 0
🧹 Nitpick comments (3)
README.md (3)
79-82: Turn the bare URL into a Markdown link and tighten phrasingUsing a naked URL violates MD034 (“no-bare-urls”) and is less readable than an inline link.
-See https://stackblitz.com/edit/unrs-resolver for usage example. +See [interactive example on StackBlitz](https://stackblitz.com/edit/unrs-resolver) for a usage demonstration.
227-235: Add a language hint to the fenced blockMarkdown-lint flags this block with MD040 because the language is omitted.
-``` +```text 2024-06-11T07:12:20.003537Z DEBUG unrs_resolver: options: ResolveOptions { ... }, path: "...", specifier: "...", ret: "..." at /path/to/unrs_resolver-1.8.1/src/lib.rs:212 in unrs_resolver::resolve with path: "...", specifier: "..."--- `242-243`: **Specify the shell language for the env-var example** Same MD040 issue as above; adding `bash` (or `sh`) improves readability and lint compliance. ```diff -``` +```bash UNRS_LOG=DEBUG your_program</blockquote></details> </blockquote></details> <details> <summary>📜 Review details</summary> **Configuration used: CodeRabbit UI** **Review profile: CHILL** **Plan: Pro** <details> <summary>📥 Commits</summary> Reviewing files that changed from the base of the PR and between 824844d23f7fa759da51ab9e9e7d1533969a14bf and 1b76e4da0b55ce8a00429c962786eaece14cd823. </details> <details> <summary>📒 Files selected for processing (5)</summary> * `README.md` (3 hunks) * `fuzz/Cargo.toml` (1 hunks) * `fuzz/fuzz_targets/resolver.rs` (1 hunks) * `napi/src/tracing.rs` (1 hunks) * `src/lib.rs` (1 hunks) </details> <details> <summary>🧰 Additional context used</summary> <details> <summary>🧠 Learnings (6)</summary> <details> <summary>📓 Common learnings</summary>Learnt from: JounQin
PR: #72
File: fixtures/tsconfig/cases/dot-alias/tsconfigs/tsconfig.base.json:0-0
Timestamp: 2025-04-21T06:11:46.964Z
Learning: JounQin's fix for the dot alias issue in PR #72 addresses the problem by correctly normalizing paths in theextend_tsconfigmethod, ensuring that base URLs are properly resolved as absolute paths when extending tsconfigs, rather than adding special case handling for dot aliases in the resolver.Learnt from: JounQin
PR: #129
File: src/tests/pnp.rs:6-6
Timestamp: 2025-06-02T15:57:50.888Z
Learning: In the unrs-resolver project, std::env is acceptable for test cases even when deprecated, as the user prefers simpler approaches in test code compared to production code standards.Learnt from: JounQin
PR: #75
File: src/tests/pnp.rs:116-133
Timestamp: 2025-04-21T08:17:34.175Z
Learning: Test cases in unrs-resolver that intentionally document failing behavior should make the intent clear by either: 1) asserting on the expected error condition rather than the desired behavior, 2) using #[should_panic] for tests expected to panic, or 3) using #[ignore] with a reason to skip known failing tests in CI.</details> <details> <summary>fuzz/Cargo.toml (4)</summary>Learnt from: JounQin
PR: #136
File: src/tests/extension_alias.rs:36-46
Timestamp: 2025-06-05T18:52:30.363Z
Learning: The fixtures/yarn directory in the unrs-resolver project requires runningyarn installto install TypeScript dependencies before tests can run, similar to other fixture directories like fixtures/pnp. The test in src/tests/extension_alias.rs depends on the installed TypeScript package providing lib/typescript.d.ts and related files.Learnt from: JounQin
PR: #136
File: src/tests/extension_alias.rs:36-46
Timestamp: 2025-06-05T18:52:30.363Z
Learning: The fixtures/yarn directory in the unrs-resolver project requires runningyarn installto install TypeScript dependencies before tests can run, similar to other fixture directories like fixtures/pnp. The test in src/tests/extension_alias.rs depends on the installed TypeScript package providing lib/typescript.d.ts and related files.Learnt from: JounQin
PR: #75
File: src/tests/pnp.rs:116-133
Timestamp: 2025-04-21T08:13:59.358Z
Learning: In the unrs-resolver project, tests that intentionally document failing behavior (regression tests) should either use #[should_panic], assert on the expected error, or include clear documentation indicating they are intentional failing test cases.Learnt from: JounQin
PR: #75
File: src/tests/pnp.rs:116-133
Timestamp: 2025-04-21T08:17:34.175Z
Learning: Test cases in unrs-resolver that intentionally document failing behavior should make the intent clear by either: 1) asserting on the expected error condition rather than the desired behavior, 2) using #[should_panic] for tests expected to panic, or 3) using #[ignore] with a reason to skip known failing tests in CI.</details> <details> <summary>README.md (11)</summary>Learnt from: JounQin
PR: #136
File: src/tests/extension_alias.rs:36-46
Timestamp: 2025-06-05T18:52:30.363Z
Learning: The fixtures/yarn directory in the unrs-resolver project requires runningyarn installto install TypeScript dependencies before tests can run, similar to other fixture directories like fixtures/pnp. The test in src/tests/extension_alias.rs depends on the installed TypeScript package providing lib/typescript.d.ts and related files.Learnt from: JounQin
PR: #136
File: src/tests/extension_alias.rs:36-46
Timestamp: 2025-06-05T18:52:30.363Z
Learning: The fixtures/yarn directory in the unrs-resolver project requires runningyarn installto install TypeScript dependencies before tests can run, similar to other fixture directories like fixtures/pnp. The test in src/tests/extension_alias.rs depends on the installed TypeScript package providing lib/typescript.d.ts and related files.Learnt from: JounQin
PR: #40
File: src/tests/resolve.rs:121-136
Timestamp: 2025-03-29T04:14:03.715Z
Learning: For tests involving npm/yarn package resolution in the unrs-resolver project, dependencies must be installed first using the appropriate package manager commands (pnpm i, yarn) in both the root directory and fixture directories like fixtures/pnp and fixtures/misc/nested-package-json.Learnt from: JounQin
PR: #75
File: src/tests/pnp.rs:116-133
Timestamp: 2025-04-21T09:52:45.840Z
Learning: In Yarn PnP environments, some packages use nested package.json files for subpaths instead of the exports field. The resolver should check for these nested package.json files when resolving subpaths like@atlaskit/pragmatic-drag-and-drop/combineto correctly handle packages with this structure.Learnt from: JounQin
PR: #129
File: src/tests/pnp.rs:6-6
Timestamp: 2025-06-02T15:57:50.888Z
Learning: In the unrs-resolver project, std::env is acceptable for test cases even when deprecated, as the user prefers simpler approaches in test code compared to production code standards.Learnt from: JounQin
PR: #75
File: src/tests/pnp.rs:116-133
Timestamp: 2025-04-21T08:17:34.175Z
Learning: Test cases in unrs-resolver that intentionally document failing behavior should make the intent clear by either: 1) asserting on the expected error condition rather than the desired behavior, 2) using #[should_panic] for tests expected to panic, or 3) using #[ignore] with a reason to skip known failing tests in CI.Learnt from: JounQin
PR: #72
File: fixtures/tsconfig/cases/dot-alias/tsconfigs/tsconfig.base.json:4-6
Timestamp: 2025-04-20T16:00:16.106Z
Learning: The configuration with".": ["."]in the paths setting in fixtures/tsconfig/cases/dot-alias/tsconfigs/tsconfig.base.json is an intentional test case for testing the TypeScript dot alias resolution behavior, specifically related to issue #437 in the eslint-import-resolver-typescript repository.Learnt from: JounQin
PR: #75
File: src/tests/pnp.rs:116-133
Timestamp: 2025-04-21T08:13:59.358Z
Learning: In the unrs-resolver project, tests that intentionally document failing behavior (regression tests) should either use #[should_panic], assert on the expected error, or include clear documentation indicating they are intentional failing test cases.Learnt from: JounQin
PR: #144
File: napi/patch.mjs:5-5
Timestamp: 2025-06-10T13:53:02.983Z
Learning: Build-time scripts in napi/patch.mjs and similar build pipeline contexts don't require error handling for reading expected files like index.js, as these files are guaranteed to be present during the build process.Learnt from: JounQin
PR: #72
File: fixtures/tsconfig/cases/dot-alias/tsconfigs/tsconfig.base.json:0-0
Timestamp: 2025-04-21T06:11:46.964Z
Learning: JounQin's fix for the dot alias issue in PR #72 addresses the problem by correctly normalizing paths in theextend_tsconfigmethod, ensuring that base URLs are properly resolved as absolute paths when extending tsconfigs, rather than adding special case handling for dot aliases in the resolver.Learnt from: JounQin
PR: #115
File: tests/resolve_test.rs:251-263
Timestamp: 2025-05-28T02:21:41.547Z
Learning: In pnpm workspaces,pnpm installmust be run first to initialize dependencies and create the proper node_modules/.pnpm structure before tests can pass. Tests that rely on resolving packages will fail if dependencies haven't been installed yet.</details> <details> <summary>napi/src/tracing.rs (2)</summary>Learnt from: JounQin
PR: #129
File: src/tests/pnp.rs:6-6
Timestamp: 2025-06-02T15:57:50.888Z
Learning: In the unrs-resolver project, std::env is acceptable for test cases even when deprecated, as the user prefers simpler approaches in test code compared to production code standards.Learnt from: JounQin
PR: #75
File: src/tests/pnp.rs:116-133
Timestamp: 2025-04-21T08:17:34.175Z
Learning: Test cases in unrs-resolver that intentionally document failing behavior should make the intent clear by either: 1) asserting on the expected error condition rather than the desired behavior, 2) using #[should_panic] for tests expected to panic, or 3) using #[ignore] with a reason to skip known failing tests in CI.</details> <details> <summary>src/lib.rs (5)</summary>Learnt from: JounQin
PR: #136
File: src/tests/extension_alias.rs:36-46
Timestamp: 2025-06-05T18:52:30.363Z
Learning: The fixtures/yarn directory in the unrs-resolver project requires runningyarn installto install TypeScript dependencies before tests can run, similar to other fixture directories like fixtures/pnp. The test in src/tests/extension_alias.rs depends on the installed TypeScript package providing lib/typescript.d.ts and related files.Learnt from: JounQin
PR: #136
File: src/tests/extension_alias.rs:36-46
Timestamp: 2025-06-05T18:52:30.363Z
Learning: The fixtures/yarn directory in the unrs-resolver project requires runningyarn installto install TypeScript dependencies before tests can run, similar to other fixture directories like fixtures/pnp. The test in src/tests/extension_alias.rs depends on the installed TypeScript package providing lib/typescript.d.ts and related files.Learnt from: JounQin
PR: #75
File: src/tests/pnp.rs:116-133
Timestamp: 2025-04-21T08:13:59.358Z
Learning: In the unrs-resolver project, tests that intentionally document failing behavior (regression tests) should either use #[should_panic], assert on the expected error, or include clear documentation indicating they are intentional failing test cases.Learnt from: JounQin
PR: #75
File: src/tests/pnp.rs:116-133
Timestamp: 2025-04-21T08:17:34.175Z
Learning: Test cases in unrs-resolver that intentionally document failing behavior should make the intent clear by either: 1) asserting on the expected error condition rather than the desired behavior, 2) using #[should_panic] for tests expected to panic, or 3) using #[ignore] with a reason to skip known failing tests in CI.Learnt from: JounQin
PR: #40
File: src/tests/resolve.rs:121-136
Timestamp: 2025-03-29T04:14:03.715Z
Learning: For tests involving npm/yarn package resolution in the unrs-resolver project, dependencies must be installed first using the appropriate package manager commands (pnpm i, yarn) in both the root directory and fixture directories like fixtures/pnp and fixtures/misc/nested-package-json.</details> <details> <summary>fuzz/fuzz_targets/resolver.rs (5)</summary>Learnt from: JounQin
PR: #136
File: src/tests/extension_alias.rs:36-46
Timestamp: 2025-06-05T18:52:30.363Z
Learning: The fixtures/yarn directory in the unrs-resolver project requires runningyarn installto install TypeScript dependencies before tests can run, similar to other fixture directories like fixtures/pnp. The test in src/tests/extension_alias.rs depends on the installed TypeScript package providing lib/typescript.d.ts and related files.Learnt from: JounQin
PR: #136
File: src/tests/extension_alias.rs:36-46
Timestamp: 2025-06-05T18:52:30.363Z
Learning: The fixtures/yarn directory in the unrs-resolver project requires runningyarn installto install TypeScript dependencies before tests can run, similar to other fixture directories like fixtures/pnp. The test in src/tests/extension_alias.rs depends on the installed TypeScript package providing lib/typescript.d.ts and related files.Learnt from: JounQin
PR: #75
File: src/tests/pnp.rs:116-133
Timestamp: 2025-04-21T08:17:34.175Z
Learning: Test cases in unrs-resolver that intentionally document failing behavior should make the intent clear by either: 1) asserting on the expected error condition rather than the desired behavior, 2) using #[should_panic] for tests expected to panic, or 3) using #[ignore] with a reason to skip known failing tests in CI.Learnt from: JounQin
PR: #75
File: src/tests/pnp.rs:116-133
Timestamp: 2025-04-21T08:13:59.358Z
Learning: In the unrs-resolver project, tests that intentionally document failing behavior (regression tests) should either use #[should_panic], assert on the expected error, or include clear documentation indicating they are intentional failing test cases.Learnt from: JounQin
PR: #129
File: src/tests/pnp.rs:6-6
Timestamp: 2025-06-02T15:57:50.888Z
Learning: In the unrs-resolver project, std::env is acceptable for test cases even when deprecated, as the user prefers simpler approaches in test code compared to production code standards.</details> </details><details> <summary>🧬 Code Graph Analysis (1)</summary> <details> <summary>fuzz/fuzz_targets/resolver.rs (1)</summary><blockquote> <details> <summary>benches/resolver.rs (1)</summary> * `unrs_resolver` (104-144) </details> </blockquote></details> </details><details> <summary>🪛 markdownlint-cli2 (0.17.2)</summary> <details> <summary>README.md</summary> 81-81: Bare URL used (MD034, no-bare-urls) --- 231-231: Fenced code blocks should have a language specified (MD040, fenced-code-language) --- 241-241: Fenced code blocks should have a language specified (MD040, fenced-code-language) </details> </details> </details> <details> <summary>⏰ Context from checks skipped due to timeout of 90000ms (1)</summary> * GitHub Check: Socket Security: Pull Request Alerts </details> <details> <summary>🔇 Additional comments (5)</summary><blockquote> <details> <summary>src/lib.rs (1)</summary> `5-5`: **LGTM! Documentation URLs updated correctly.** The crate and npm package URLs have been properly updated to reflect the renaming from `oxc_resolver` to `unrs_resolver`. </details> <details> <summary>fuzz/Cargo.toml (1)</summary> `23-23`: **LGTM! Fuzzing dependency updated correctly.** The dependency name has been properly updated from `oxc_resolver` to `unrs_resolver` while maintaining the correct local path reference. </details> <details> <summary>fuzz/fuzz_targets/resolver.rs (1)</summary> `4-4`: **LGTM! Import updated to match renamed crate.** The import statement correctly reflects the dependency name change from `oxc_resolver` to `unrs_resolver`. </details> <details> <summary>napi/src/tracing.rs (1)</summary> `7-8`: **LGTM! Environment variable references updated consistently.** The documentation comment and environment variable lookup have been correctly updated from `OXC_LOG` to `UNRS_LOG`, maintaining consistency between documentation and implementation. Also applies to: 15-15 </details> <details> <summary>README.md (1)</summary> `263-268`: **Paths updated correctly to `crates/unrs_resolver`** The test-case references now match the crate rename; no further action required. </details> </blockquote></details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
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 8f62d75 in 57 seconds. Click for details.
- Reviewed
80lines of code in3files - Skipped
0files when reviewing. - Skipped posting
5draft 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:76
- Draft comment:
Added 'Supports WASM' section looks good. Consider adding a brief explanation of what extra setup (if any) is needed when using WASM, in addition to the StackBlitz link. - Reason this comment was not posted:
Confidence changes required:33%<= threshold50%None
2. README.md:227
- Draft comment:
Updated debug log example now correctly references 'unrs_resolver::resolve'. Verify that all similar logging/documentation is consistent throughout the docs. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
3. README.md:255
- Draft comment:
Test case references have been updated to point to the new 'unrs_resolver' crate paths. Ensure that all related links and references in the docs are updated accordingly. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
4. fuzz/Cargo.toml:4
- Draft comment:
Edition updated to '2024'. Please double-check that your toolchain supports this edition as it's not a widely used stable edition yet. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%The comment is asking the PR author to double-check their toolchain compatibility with the updated edition. This violates the rule against asking the author to double-check things.
5. fuzz/fuzz_targets/resolver.rs:4
- Draft comment:
Resolver import updated to 'unrs_resolver::Resolver' which is now consistent with the new package name. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
Workflow ID: wflow_swNCUCwvb0ufqKKb
You can customize by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.
|
Please retry analysis of this Pull-Request directly on SonarQube Cloud |
Important
Update references to
unrs_resolver, add WASM example, and update Rust edition inCargo.toml.unrs_resolver.README.md.README.md.README.md.unrs_resolverinfuzz/fuzz_targets/resolver.rsandfuzz/Cargo.toml.fuzz/Cargo.toml.This description was created by
for 8f62d75. You can customize this summary. It will automatically update as commits are pushed.
Summary by CodeRabbit
Documentation
Chores