Skip to content
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.

Sign up for GitHub

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

fix: filePath with "#" is a directory #2432

Closed
wants to merge 8 commits into from
Closed

fix: filePath with "#" is a directory #2432

Changes from 5 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
57893b5
fix: filePath with "#" is a direction
YufJi Mar 8, 2021
ffd4c0d
fix: optimization code
YufJi Mar 31, 2021
2060f21
fix: remove useless condition
YufJi Apr 1, 2021
0e38265
fix: bug
YufJi Apr 1, 2021
0fb5172
Apply suggestions from code review
Shinigami92 Apr 1, 2021
1a6e643
Merge branch 'main' into fix--filePath-with-'#'-is-a-direction
YufJi Apr 2, 2021
1121768
chore: merge main into pr
YufJi Apr 2, 2021
388dbef
Merge branch 'fix--filePath-with-'#'-is-a-direction' of https://githu…
YufJi Apr 2, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
46 changes: 32 additions & 14 deletions packages/vite/src/node/plugins/resolve.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -222,23 +222,12 @@ export function resolvePlugin(baseOptions: InternalResolveOptions): Plugin {
}
}

function tryFsResolve(
fsPath: string,
function tryResolve(
file: string,
postfix: string,
options: InternalResolveOptions,
tryIndex = true
): string | undefined {
let file = fsPath
let postfix = ''

let postfixIndex = fsPath.indexOf('?')
if (postfixIndex < 0) {
postfixIndex = fsPath.indexOf('#')
Copy link
Contributor

@hyrious hyrious Jul 11, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

As #2346 (comment) said, can we just remove this line so that # are always treated as part of file path?

}
if (postfixIndex > 0) {
file = fsPath.slice(0, postfixIndex)
postfix = fsPath.slice(postfixIndex)
}

let res: string | undefined
for (const ext of options.extensions || DEFAULT_EXTENSIONS) {
if (
Expand All @@ -261,6 +250,35 @@ function tryFsResolve(
}
}

function tryFsResolve(
fsPath: string,
options: InternalResolveOptions,
tryIndex = true
): string | undefined {
let file = fsPath
let postfix = ''
let res: string | undefined

// 1、Try resolving without removing postfixes first
if ((res = tryResolve(file, postfix, options, tryIndex))) {
Copy link
Member

@patak-dev patak-dev Apr 2, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

We are going to lose the optimization introduced in #2718 with this PR.

When people use imports with extensions, we should first check for an exact match before trying the list of auto-resolved extensions. Also, I think that we should first with the extensions removed to avoid two checks per file for common cases.

The order of the tryResolveFile calls should be:

  file ( tryIndex: false )
  file ( tryIndex: false, removing postfixes )

  for each ext
    file+ext
    file+ext ( removing postfixes ) 

  file ( tryIndex: true )
  file ( tryIndex: true, removing postfixes )

I also think that if ? and # are inside the folder path and are not part of the file, we can safely remove the postfixes so we avoid the double checks. So we should check for the presence of ? and # only after the last /. This change alone should fix most issues (the other changes in this PR are still important though).

return res
}

// 2、If that didn't work, treat ? and # as postfixes, remove them and try resolving again.
let postfixIndex = fsPath.indexOf('?')
if (postfixIndex < 0) {
postfixIndex = fsPath.indexOf('#')
}
if (postfixIndex > 0) {
file = fsPath.slice(0, postfixIndex)
postfix = fsPath.slice(postfixIndex)
}

if ((res = tryResolve(file, postfix, options, tryIndex))) {
return res
}
}

function tryResolveFile(
file: string,
postfix: string,
Expand Down