perf(resolve): support # in path only for dependencies #12469
Merged
Conversation
This file contains 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
|
|
patak-dev
changed the title
|
/ecosystem-ci run |
patak-dev
added
performance
|
📝 Ran ecosystem CI: Open
|
|
Ecosystem CI looks fine, same fails as in main (SvelteKit latest main is failing rigth now) |
bluwy
reviewed
Mar 17, 2023
bluwy
previously approved these changes
Mar 17, 2023
sun0day
reviewed
Mar 18, 2023
| let res: string | undefined | ||
|
|
||
| // if there is a postfix, try resolving it as a complete path first (#4703) | ||
| if ( |
There was a problem hiding this comment.
I think we can add some comments before each if branch. The comments tell what fsPath will hit these if conditions. It's helpful for understanding these codes quickly and refactoring them later. And I guess there are still rooms to dedupe resolve logic.
There was a problem hiding this comment.
I renamed the variable instead of adding a new comment because I think the main comment for tryUnsplitted is enough now.
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.
Description
@sun0day has been investigating
tryFsResolvedouble checks to support paths with#. See:We added the checks to resolve the file path without removing the postfix because of dependencies like es5-ext here:
But the use of
?and#never worked in user code and this is something we can control. Requiring users to avoid?and#in their paths is a fair price to pay to avoid double checks.This PR formalizes that we only support
#in dependencies paths, and removes the double checks in other cases. Note: I only included support for#here, and not?so the double checks won't be triggered for every dependency.What is the purpose of this pull request?