Skip to content

fix: dont resolve imports with malformed URI#16244

Merged
patak-cat merged 2 commits intovitejs:mainfrom
sapphi-red:fix/dont-resolve-percent-without-escaped
Mar 23, 2024
Merged

fix: dont resolve imports with malformed URI#16244
patak-cat merged 2 commits intovitejs:mainfrom
sapphi-red:fix/dont-resolve-percent-without-escaped

Conversation

@sapphi-red
Copy link
Copy Markdown
Member

@sapphi-red sapphi-red commented Mar 23, 2024

Description

To align with native ESM, Vite should not resolve import './foo%.js' to ./foo%.js (#15298 (comment)).
repro with node: https://stackblitz.com/edit/node-ccxwu6?file=index.js

Additional context

What is the purpose of this pull request?

  • Bug fix
  • New Feature
  • Documentation update
  • Other

Before submitting the PR, please make sure you do the following

  • Read the Contributing Guidelines, especially the Pull Request Guidelines.
  • Check that there isn't already a PR that solves the problem the same way to avoid creating a duplicate.
  • Provide a description in this PR that addresses what the PR is solving, or reference the issue that it solves (e.g. fixes #123).
  • Update the corresponding documentation if needed.
  • Ideally, include relevant tests that fail without this PR but pass with it.

@sapphi-red sapphi-red added p3-minor-bug An edge case that only affects very specific usage (priority) regression The issue only appears after a new release labels Mar 23, 2024
@bolt-new-by-stackblitz
Copy link
Copy Markdown

bolt-new-by-stackblitz Bot commented Mar 23, 2024

Review PR in StackBlitz Codeflow Run & review this pull request in StackBlitz Codeflow.

sapphi-red
sapphi-red commented Mar 23, 2024
View reviewed changes
Comment thread playground/assets/nested/テスト-測試-white space.png Outdated
Copy link
Copy Markdown
Member Author

@sapphi-red sapphi-red Mar 23, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

vite/playground/assets/index.html

Lines 535 to 552 in 6c8c7b8

function testDynamicImportMetaUrlWithQuery(name, i) {
// prettier-ignore
const metaUrl = new URL(`./nested/${name}.png?abc`, import.meta.url,)
text(`.dynamic-import-meta-url-${i}-query`, metaUrl)
document.querySelector(`.dynamic-import-meta-url-img-${i}-query`).src =
metaUrl
}
testDynamicImportMetaUrlWithQuery('icon', 1)
testDynamicImportMetaUrlWithQuery('asset', 2)
function testDynamicImportMetaUrlWithTernaryOperator(name, i) {
// prettier-ignore
const metaUrl = new URL(`./nested/${1 === 0 ? 'failed' : name}.png?abc`, import.meta.url,)
text(`.dynamic-import-meta-url-${i}-ternary`, metaUrl)
document.querySelector(`.dynamic-import-meta-url-img-${i}-ternary`).src =
metaUrl
}

These new URLs is transformed to import.meta.glob and then it is transformed to import url from './nested/テスト-測試-white space%.png'. These should be import url from './nested/テスト-測試-white space%25.png', but that change opens up a can of worms (#16243), so I simply renamed back these files and put a different file in a different directory.

@sapphi-red
Copy link
Copy Markdown
Member Author

sapphi-red commented Mar 23, 2024

/ecosystem-ci run

@vite-ecosystem-ci
Copy link
Copy Markdown

vite-ecosystem-ci Bot commented Mar 23, 2024

📝 Ran ecosystem CI on e88ceee: Open

✅ analogjs, astro, histoire, ladle, laravel, marko, nuxt, previewjs, quasar, qwik, rakkas, remix, sveltekit, unocss, vike, vite-plugin-pwa, vite-plugin-react, vite-plugin-react-pages, vite-plugin-react-swc, vite-plugin-svelte, vite-plugin-vue, vite-setup-catalogue, vitepress, vitest

@patak-cat patak-cat merged commit fbf69d5 into vitejs:main Mar 23, 2024
@sapphi-red sapphi-red deleted the fix/dont-resolve-percent-without-escaped branch March 23, 2024 10:16
@clicktodev
Copy link
Copy Markdown
Contributor

clicktodev commented Dec 29, 2024 •
edited
Loading

hey i know this was meant to fix #15298 but the issue still persists in vite 6.
"vite": "^6.0.5"
logo-10%.png
in dev: URI malformed error
in production build: no errors and file is resolved correctly

dev:
image

prod build:
image

@sapphi-red sapphi-red mentioned this pull request Dec 30, 2024
Open
7 tasks
@sapphi-red
Copy link
Copy Markdown
Member Author

sapphi-red commented Dec 30, 2024

@hichemfantar I reopened #15298 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@patak-cat patak-cat patak-cat approved these changes

Assignees

No one assigned

Labels

p3-minor-bug An edge case that only affects very specific usage (priority) regression The issue only appears after a new release

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@sapphi-red @clicktodev @patak-cat