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.

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
2 participants