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(plugin-legacy): empty base makes import fail (fixes #4212) #8387

Merged
merged 1 commit into from
May 29, 2022

Conversation

sapphi-red
Copy link
Member

Description

fixes #4212

I tested with Chrome 53.

Additional context

Maybe '' could be replaced to './' in resolveConfig instead?


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 Commit 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 plugin: legacy p2-edge-case Bug, but has workaround or limited in scope (priority) labels May 29, 2022
@patak-dev patak-dev merged commit 1a16f12 into vitejs:main May 29, 2022
@patak-dev
Copy link
Member

patak-dev commented May 29, 2022

Maybe '' could be replaced to './' in resolveConfig instead?

Yes, this is an option. We'll need to discuss with the team if we want to deprecate both '' and './' in favor of 'auto', or if we decide for '' or './' and deprecate with a warning or an error in v3 the other one. I would prefer to have a single option, and lately, I'm leaning towards '', false, or null. I think the relative base not being a string is better to avoid future errors as it will force us to always check the base type.

@sapphi-red sapphi-red deleted the fix/legacy-base-empty branch May 29, 2022 23:55
@sapphi-red
Copy link
Member Author

I'm in favor of having a single option. null could be confusing with omitting the value. So I think false is better if we are going to using a type other than string.

@60late
Copy link

60late commented Nov 27, 2024

this problem still not fixed in v5.4.3
I set my base option like base: /abc/def/ , and use plugin-legacy, it will lead to errors like:
Uncaught (in promise) Error: assets/index-legacy.d57e0ebc.js
finally i have to change the base to ./, finally solve the problem. But i have to use legacy files in all browser -- morden or old.
Can this problem be solved?
#4212

@patak-dev
Copy link
Member

@60late please create a new issue with a minimal reproduction against vite latest version. Thanks!

@60late
Copy link

60late commented Dec 6, 2024

@60late please create a new issue with a minimal reproduction against vite latest version. Thanks!

After some tests on this problem. I found it was solved when i use vite: 5.4.11. I think it may caused by error vite version.(I used v.4.4.1)
For anyone who may need, upgrade your vite version will solve this problem

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
p2-edge-case Bug, but has workaround or limited in scope (priority) plugin: legacy
Projects
None yet
Development

Successfully merging this pull request may close these issues.

plugin-legacy SystemJS https://git.io/JvFET#8
3 participants