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.

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

File import containing % does not resolve correctly #15298

Open
7 tasks done
hichemfantar opened this issue Dec 10, 2023 · 8 comments · Fixed by #15311
Open
7 tasks done

File import containing % does not resolve correctly #15298

hichemfantar opened this issue Dec 10, 2023 · 8 comments · Fixed by #15311
Labels
p3-minor-bug An edge case that only affects very specific usage (priority)

Comments

@hichemfantar
Copy link
Contributor

Describe the bug

"Internal server error: URI malformed" when using a percent sign in URL

Reproduction

https://github.com/hichemfantar/vite-bug-repro

Steps to reproduce

  1. create a file with % in name
  2. import it

System Info

System:
    OS: Linux 5.15 Ubuntu 20.04.6 LTS (Focal Fossa)
    CPU: (12) x64 Intel(R) Core(TM) i5-10500H CPU @ 2.50GHz
    Memory: 9.71 GB / 15.40 GB
    Container: Yes
    Shell: 5.0.17 - /bin/bash
  Binaries:
    Node: 18.18.2 - ~/.nvm/versions/node/v18.18.2/bin/node
    Yarn: 1.22.19 - ~/.nvm/versions/node/v18.18.2/bin/yarn
    npm: 9.8.1 - ~/.nvm/versions/node/v18.18.2/bin/npm
    pnpm: 8.10.2 - ~/.nvm/versions/node/v18.18.2/bin/pnpm
  Browsers:
    Chrome: 118.0.5993.88
  npmPackages:
    @vitejs/plugin-react: ^4.2.0 => 4.2.1 
    vite: ^5.0.0 => 5.0.7

Used Package Manager

npm

Logs

no build errors

Validations

@benmccann benmccann added bug p3-minor-bug An edge case that only affects very specific usage (priority) p3-downstream-blocker Blocking the downstream ecosystem to work properly (priority) and removed pending triage labels Dec 10, 2023
@benmccann
Copy link
Collaborator

benmccann commented Dec 10, 2023

I'm unsure if the handling of % is a bug. However users of vite-imagetools are reporting that a space in the filename doesn't work, which definitely seems like a bug: JonasKruckenberg/imagetools#670

@benmccann benmccann removed the p3-downstream-blocker Blocking the downstream ecosystem to work properly (priority) label Dec 11, 2023
@sapphi-red
Copy link
Member

sapphi-red commented Dec 12, 2023

Faux ESM treats the import specifier (I mean the bar.js part of import foo from './bar.js') as a file path and native ESM (Node/browsers/deno) treats it as URL.

For example, in order to import a file named %foo, the import specifier is different between them.
Faux ESM: import foo from './%foo'
Native ESM: import foo from './%25foo'

I guess we first need to decide how Vite should handle this.

@benmccann
Copy link
Collaborator

What is faux ESM? Do you mean how it's treated by Vite during bundling? We can't change how native ESM handles it, so I imagine that if we wanted to fix this that our only choice would be to align with native ESM.

@sapphi-red
Copy link
Member

What is faux ESM? Do you mean how it's treated by Vite during bundling?

It's a term that refers to files that cannot be run in the browser or Node.js, but use ESM import/export. It's consumed by a bundler including Vite, Webpack, parcel, etc. For example, a file is specified by module field is sometimes faux ESM. I believe there won't be many packages importing a file that requires URL encoding, but it does have a chance to break some packages if we change the way.

If we align the behavior with native ESM, this error itself is not a bug. (We could add a more friendly error message though.) It seems import SmallLogo from "./assets/logo-10%25.png"; doesn't work and that is a bug.

@patak-dev patak-dev removed the bug label Feb 10, 2024
bluwy added a commit that referenced this issue Mar 12, 2024
@github-actions github-actions bot locked and limited conversation to collaborators Mar 27, 2024
@sapphi-red sapphi-red reopened this Dec 30, 2024
@vitejs vitejs unlocked this conversation Dec 30, 2024
@sapphi-red
Copy link
Member

Reopening as the fix by #15311 was not correct and #16244 reverted that part.

To clarify, the error for the import './assets/logo-10%.png' is correct and what we need to fix is that ./assets/logo-10%25.png not resolving to ./assets/logo-10%.png (see #15298 (comment) for the reason).

@sapphi-red sapphi-red changed the title "Internal server error: URI malformed" when using a percent sign in URL imports containing URL encoded characters does not resolve correctly Dec 30, 2024
@sapphi-red
Copy link
Member

Note that TypeScript does not work well in these cases (microsoft/TypeScript#41730).

@hi-ogawa
Copy link
Collaborator

Is this issue also related #9917? The issue description says file:// and that's probably supported now, but the repro is about import("./test%20utils/helper.mjs") not being resolved to ./test utils/helper.mjs while nodejs supports it https://github.com/clarkdo/vest-import-with-spaces-issue/blob/73be84495637538af2c2287129a7ce2ab2551ee2/index.test.mjs#L4C30-L4C55

@sapphi-red
Copy link
Member

Ah, yeah, it seems the root cause is same.

@hichemfantar hichemfantar changed the title imports containing URL encoded characters does not resolve correctly File import containing % does not resolve correctly Dec 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
p3-minor-bug An edge case that only affects very specific usage (priority)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants