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

Type incompatibilities between mergeConfig and defineConfig when the latter is used with a function #13950

Closed
7 tasks done
vinassefranche opened this issue Jul 25, 2023 · 8 comments · Fixed by #13958
Closed
7 tasks done
Labels
enhancement New feature or request

Comments

@vinassefranche
Copy link
Contributor

vinassefranche commented Jul 25, 2023

Describe the bug

I am creating a config with defineConfig by providing a function. When I want to use this function in mergeConfig, I get a TypeScript error:
Argument of type 'UserConfigExport' is not assignable to parameter of type 'UserConfig | Promise<UserConfig>'. Type 'UserConfigFnObject' is not assignable to type 'UserConfig | Promise<UserConfig>'.

Here's the code that's in the typescript playground added as a reproduction. The function in defineConfig is a dummy function here to showcase the issue. I know that here, I could get rid of the function but in my real repository, I need to access the command information

import { defineConfig, mergeConfig } from 'vite';

// Does not work when using a function parameter
const config = defineConfig(() => ({
    plugins: [],
}));
    
mergeConfig(
  config,
  defineConfig({
    envPrefix: 'PUBLIC_',
  }),
);

// Works when not using a function parameter
const config2 = defineConfig({
    plugins: [],
});

mergeConfig(
  config2,
  defineConfig({
    envPrefix: 'PUBLIC_',
  }),
);

The issue is that mergeConfig does not accept functions (see D extends Function ? never : D, in

export function mergeConfig<
) so UserConfigExport is not an acceptable type for it.

Is there something I'm doing wrong or is defineConfig not defined as it should be?

Reproduction

https://www.typescriptlang.org/play#code/JYWwDg9gTgLgBAbzgEwKYDNgDtUGEJaYDmANHCKlEXgcXAL5zpQQhwDkAbsDKuwNwAoQQHoRcACIRUAZzhYI8AO7QA1nCUALVFjgBXGdiJwAhkz1YAxjGAE4YE1BMVeUQZYIz4HwsGMBeFAxsGl8iAApwgEo4fwA+OHCEQThU+wAbPSJsGQAuOABtAF0SQXooqKE0uEEKKlDicJS4H2JS1LRMHHwwpObUnU4ABShggA989iGAVQAhABkASVwAfXZ2hijSyuExOAB1NTktHXlFfUMsYzN0C2tbXQcnF0p3T29aPwAmWKCuhr8fWqYEy2SweUKJTKO1qlGoPUazVa3w2nRCCMByWqgxG40mMwWyzWG3K234QA

Steps to reproduce

No response

System Info

vitest@4.4.7

Used Package Manager

yarn

Logs

No response

Validations

@patak-dev
Copy link
Member

This is expected. See #13135 (and the linked PR from it). You can call the config function before passing it to mergeConfig.

@patak-dev patak-dev closed this as not planned Won't fix, can't repro, duplicate, stale Jul 25, 2023
@vinassefranche
Copy link
Contributor Author

vinassefranche commented Jul 25, 2023

Thanks for your answer!

I'm not sure I understand the workaround, to be honest. I have 2 issues:

  1. config is not callable
    I can't just call config as Typescript prevents me from calling it (rightfully, as it's a union type of types that are not all functions)
    Maybe defineConfig should have more overloads to be able to discriminate more the result type.
    Why not use a generic for this, by the way?

    export function defineConfig<Config extends UserConfigExport>(config: Config): Config {
      return config
    }
  2. Even if config was callable, I'm supposed to pass an env parameter to it, right?
    Should I use mergeConfig in a definedConfig like this?

    defineConfig(env => mergeConfig(
      config(env),
      defineConfig({
        envPrefix: 'PUBLIC_',
      }),
    ));

Thanks again for your interest in this. Tell me if I'm not clear in what I'm asking!

@patak-dev
Copy link
Member

Let's reopen the issue to keep discussing this. I agree it would be good if defineConfig would return a function type when passing a function. PR welcome to check how things would look when we run CI against other projects in the ecosystem.

And if that is the case, your code snippet for 2 seems good to me. You can cast the function for now after the first defineConfig so you aren't blocked.

@vinassefranche
Copy link
Contributor Author

I've opened the pull request: #13958
I see that there's a defineConfig in the types of vitest with the old typings (the one before the pull request you referenced in your first post) Do you know if I need to open a pull request there too or if it's automatically back-ported from this repository?

@patak-dev
Copy link
Member

Thanks! There is no automatic process to port things to Vitest, but you could first wait until #13958 is cleared.

@Anoesj
Copy link

Anoesj commented Aug 8, 2023

Would be nice to update the documentation with the latest info. I'm glad I found this issue, otherwise I wouldn't have figured out that I could use this pattern:

import { mergeConfig, defineConfig } from 'vite';
import viteConfig from './vite.config';

export default defineConfig((env) => mergeConfig(
  viteConfig(env),
  defineConfig({
    test: {
      // vitest config
    },
  }),
));

@patak-dev
Copy link
Member

@Anoesj would you like to send a PR to the Vitest repo to do that? That is a good contribution for the project

@vinassefranche
Copy link
Contributor Author

@patak-dev I just created this pull request to add it to vite's documentation as well. Tell me if you don't think it's a good idea.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants