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

fix: call tryFsResolve for relative new URL(foo, import.meta.url) #13142

Merged
merged 3 commits into from
May 10, 2023

Conversation

sapphi-red
Copy link
Member

@sapphi-red sapphi-red commented May 10, 2023

Description

new URL('./path-without-extension', import.meta.url) was supported in past but wasn't working since 4.3. This was caused by #12450.

fixes #13141

refs #7837

Additional context

I wonder if we should remove these resolvings from new URL(foo, import.meta.url) (i.e. only support relative paths with extensions) as new URL is a constructor that simply resolves relative paths. Instead to introduce import.meta.resolve for that usage as import.meta.resolve will resolve like node and it works with Node.


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.
  • Read the Pull Request Guidelines and follow the PR Title Convention.
  • 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).
  • 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 May 10, 2023
@stackblitz
Copy link

stackblitz bot commented May 10, 2023

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

@sapphi-red sapphi-red changed the title fix: call tryFsResolve for relative new URL(foo, import.meta.url) fix: call tryFsResolve for relative new URL(foo, import.meta.url) May 10, 2023
@patak-dev
Copy link
Member

/ecosystem-ci run

@vite-ecosystem-ci
Copy link

vite-ecosystem-ci bot commented May 10, 2023

📝 Ran ecosystem CI: Open

suite result
astro ❌ failure
histoire ✅ success
iles ✅ success
ladle ✅ success
laravel ✅ success
marko ✅ success
nuxt ❌ failure
previewjs ✅ success
qwik ✅ success
rakkas ✅ success
sveltekit ✅ success
unocss ✅ success
vite-plugin-ssr ✅ success
vite-plugin-react ✅ success
vite-plugin-react-pages ✅ success
vite-plugin-react-swc ✅ success
vite-plugin-svelte ✅ success
vite-plugin-vue ✅ success
vite-setup-catalogue ✅ success
vitepress ✅ success
vitest ✅ success

@patak-dev
Copy link
Member

I wonder if we should remove these resolvings from new URL(foo, import.meta.url) (i.e. only support relative paths with extensions) as new URL is a constructor that simply resolves relative paths. Instead to introduce import.meta.resolve for that usage as import.meta.resolve will resolve like node and it works with Node.

Given that this was working in 4.2, I think it is a good idea to move forward with your PR. We can discuss the breaking change for Vite 5, but I think we may need to keep supporting this. import.meta.resolve looks like a good tool to have though.

Copy link
Member

@patak-dev patak-dev left a comment

Choose a reason for hiding this comment

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

Nuxt and Astro are also failing in main

@patak-dev patak-dev merged commit eeb0617 into vitejs:main May 10, 2023
@sapphi-red sapphi-red deleted the fix/resolve-fs-for-url-import-meta branch May 11, 2023 07:08
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) regression The issue only appears after a new release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Web worker files without extension in symlinked packages fail to load during development as of version 4.3
2 participants