fix: should only use context.physicalFilename as fallback instead of main source
#399
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
🦋 Changeset detectedLatest commit: 5f684d2 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 |
JounQin
changed the title
context.physicalFilename as fallback instead of main sourcecontext.physicalFilename as fallback instead of main source
There was a problem hiding this comment.
Pull Request Overview
This pull request fixes an issue with fallback priority when resolving source files and updates the version of the autofix CI workflow action.
- Reorders the array to prioritize the main source file over the physical filename fallback.
- Updates the autofix-ci/action version to v1.3.2 in the CI workflow.
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| src/utils/resolve.ts | Reorders fallback array to prioritize the main source file |
| .github/workflows/autofix.yml | Updates the version of autofix-ci/action to v1.3.2 |
Comments suppressed due to low confidence (2)
src/utils/resolve.ts:309
- [nitpick] Consider adding a comment to explain why the 'sourceFile' is prioritized over 'context.physicalFilename' to improve code clarity and maintainability for future reviewers.
: [sourceFile, context.physicalFilename]
.github/workflows/autofix.yml:34
- Ensure the upgraded autofix-ci/action version is compatible with the rest of the CI setup and update related documentation if necessary.
uses: autofix-ci/action@635ffb0c9798bd160680f18fd73371e355b85f27 # v1.3.2
WalkthroughThis change updates the fallback behavior in the Changes
Assessment against linked issues
Assessment against linked issues: Out-of-scope changesNo out-of-scope changes detected. Possibly related PRs
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/utils/import-type.tsOops! Something went wrong! :( ESLint: 9.29.0 Error [ERR_MODULE_NOT_FOUND]: Cannot find module '/lib/index.js' imported from /eslint.config.js src/utils/resolve.tsOops! Something went wrong! :( ESLint: 9.29.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 (2)
🚧 Files skipped from review as they are similar to previous changes (1)
🧰 Additional context used🧠 Learnings (2)📓 Common learningssrc/utils/import-type.ts (4)⏰ Context from checks skipped due to timeout of 90000ms (21)
🔇 Additional comments (1)
✨ 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 (
|
|
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. |
![ellipsis-dev[bot]](https://avatars.githubusercontent.com/in/64358?s=60&v=4){kind=small}
There was a problem hiding this comment.
Important
Looks good to me! 👍
Reviewed everything up to c0a6fc9 in 26 seconds. Click for details.
- Reviewed
25lines of code in2files - Skipped
0files when reviewing. - Skipped posting
2draft 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. .github/workflows/autofix.yml:34
- Draft comment:
Upgrade to v1.3.2 looks good. Confirm that pinning to a commit hash is acceptable for stability. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
2. src/utils/resolve.ts:307
- Draft comment:
Swapping the order of [sourceFile, context.physicalFilename] correctly prioritizes the main source. This resolves the fallback issue noted in #397. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%This comment is purely informative, explaining the reason behind a code change. It doesn't ask for any specific action or suggest any improvements. According to the rules, purely informative comments should be removed.
Workflow ID: wflow_4CLH8ntT715rJpBN
You can customize 
commit: |
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 #399 +/- ##
=======================================
Coverage 96.10% 96.10%
=======================================
Files 96 96
Lines 4951 4952 +1
Branches 1836 1862 +26
=======================================
+ Hits 4758 4759 +1
Misses 192 192
Partials 1 1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Signed-off-by: JounQin <[email protected]>
JounQin
force-pushed
the
fix/resolve_regression
branch
from
5c9afe7 to
f82cb46
Compare
SukkaW
reviewed
Jun 26, 2025
I don't know how to add that case easily. The 3 issues reported confirm it's working as expected. @SukkaW Here we go: |
There was a problem hiding this comment.
Important
Looks good to me! 👍
Reviewed 44744b7 in 1 minute and 58 seconds. Click for details.
- Reviewed
64lines of code in2files - Skipped
0files when reviewing. - Skipped posting
4draft 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/utils/resolve.ts:307
- Draft comment:
The order of sourceFiles was reversed to [context.physicalFilename, sourceFile]. The PR title and test indicate that context.physicalFilename should only be a fallback, i.e. sourceFile must take priority. Please double-check if this order is correct when both resolutions succeed. - 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 the order of sourceFiles, which violates the rule against asking the author to confirm their intention or to double-check things. The comment does not provide a specific suggestion or ask for a test to be written, which would have been acceptable.
2. src/utils/resolve.ts:358
- Draft comment:
Removal of the explicit type cast in fileExistsCache.set is minor; ensure that resolved.path is handled correctly when it’s null. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%The comment suggests ensuring that a specific behavior is handled correctly, which is not allowed. It doesn't provide a specific code suggestion or ask for a test to be written. It also doesn't point out a specific issue or ask for confirmation of an intentional change.
3. src/utils/resolve.ts:307
- Draft comment:
The for‐loop reuses the variable name 'sourceFile', shadowing the function parameter. Consider renaming it for clarity. - Reason this comment was not posted:
Comment was on unchanged code.
4. test/utils/resolve.spec.ts:836
- Draft comment:
Good new test for fallback behavior. Also consider adding a test case where both context.physicalFilename and sourceFile are valid to confirm that sourceFile takes priority as intended. - Reason this comment was not posted:
Comment looked like it was already resolved.
Workflow ID: wflow_saJn4pUJl4CxL5N0
You can customize
There was a problem hiding this comment.
Important
Looks good to me! 👍
Reviewed f80e90e in 40 seconds. Click for details.
- Reviewed
13lines of code in1files - 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/utils/resolve.ts:307
- Draft comment:
Switch candidate order so that the main sourceFile is prioritized and context.physicalFilename is used only as a fallback. - Reason this comment was not posted:
Comment did not seem useful. Confidence is useful =0%<= threshold50%This comment is purely informative, explaining a change in the order of candidate selection. It does not provide a suggestion, ask for confirmation, or highlight a potential issue.
Workflow ID: wflow_NRwhsiY6uGHjmzBg
You can customize
JounQin
force-pushed
the
fix/resolve_regression
branch
from
f80e90e to
5f684d2
Compare
There was a problem hiding this comment.
Important
Looks good to me! 👍
Reviewed 5f684d2 in 1 minute and 5 seconds. Click for details.
- Reviewed
39lines of code in2files - Skipped
0files when reviewing. - Skipped posting
2draft 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/utils/import-type.ts:286
- Draft comment:
Exporting 'isExternalLookingName' enables reuse in other modules. Confirm this export is intentional and covered by tests. - Reason this comment was not posted:
Confidence changes required:0%<= threshold50%None
2. src/utils/resolve.ts:308
- Draft comment:
The conditional now uses 'sourceFile' alone unless the module is external. Verify that the fallback order [sourceFile, context.physicalFilename] is intentional and that both scenarios are tested. - Reason this comment was not posted:
Confidence changes required:33%<= threshold50%None
Workflow ID: wflow_q84Xsnh4LDOyZEFr
You can customize
|
This PR appears to also fix a “maximum call stack size exceeded” error I encountered on the released version: errorthe error disappears on this PR version |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
close #396
close #397
close #398
We're also resolving dependencies chain inside
node_modules, for example:eslint.config.js->typescript-eslint->./config-helper(would be wrong resolved here if the originaleslint.config.jsused assourceFile)Important
Fix module resolution to prioritize
sourceFileovercontext.physicalFilenameunless the module is external.fullResolve()inresolve.tsto prioritizesourceFileovercontext.physicalFilenameunless the module is external.isExternalLookingName()to determine if a module is external.isExternalLookingName()fromimport-type.ts.resolve.spec.tsto verifysourceFiletakes priority overcontext.physicalFilename.This description was created by
for 5f684d2. You can customize this summary. It will automatically update as commits are pushed.
Summary by CodeRabbit
Summary by CodeRabbit
Bug Fixes
Documentation