-
-
Notifications
You must be signed in to change notification settings - Fork 6.2k
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(types): narrow down the return type of defineConfig
#13792
fix(types): narrow down the return type of defineConfig
#13792
Conversation
Fixes compatibility with `mergeConfig`, as reported in vuejs/create-vue#313 Before the change, `defineConfig` would return `UserConfigExport`, which is a union type. This would cause `mergeConfig` to fail, as it doesn't accept the `UserConfigFn` type in the union. (vitejs#13135) After the change, `defineConfig` returns the same type that it receives, so it would exclude `UserConfigFn` from the return type whenever possible.
Run & review this pull request in StackBlitz Codeflow. |
/ecosystem-ci run |
📝 Ran ecosystem CI: Open
|
Oops. So many broken tests… 🤦♂️ I think the |
Only Qwik is failing compared to main. Iles and Histoire are known issues. |
Update: I can confirm this is affected by the
|
Further tests show that the I also managed to use the |
Based on vitejs#13792 but with slightly more verbose function style configuration types. We can make the generic version pass the tests with some hack, but it is too permissive that it would allow users to pass in unknown fields. The overload version is much more direct and simpler. Thanks @xiaoxiangmoe for helping me debug this.
/ecosystem-ci run |
📝 Ran ecosystem CI: Open
|
Ported from vitejs/vite#13792 Fully fixes issues like vuejs/create-vue#317 and vuejs/create-vue#313 For an easy reproduction, please refer to https://github.com/sodatea/viteste-define-config-bug-repro I didn't include a test in this PR because: 1. The reproduction is a bit complicated, it only fails on the very latest Vite versions, but updating Vite would mess up the lockfile. 2. By not updating the dependencies, we can make sure that this fix is compatible with older versions of Vite, too.
Description
Fixes compatibility with
mergeConfig
, as reported in vuejs/create-vue#313Before the change,
defineConfig
would returnUserConfigExport
, which is a union type. This would causemergeConfig
to fail, as it doesn't accept theUserConfigFn
type in the union. (#13135)After the change,
defineConfig
returns the same type that it receives, so it would excludeUserConfigFn
from the return type whenever possible.Additional context
What is the purpose of this pull request?
Before submitting the PR, please make sure you do the following
fixes #123
).