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

feat: export default config #19005

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

ferferga
Copy link

@ferferga ferferga commented Dec 18, 2024

Description

Makes the default config consumable by the user.

Use case

I'm developing a plugin that needs to access the build.outDir variable:

config: (conf) => {
    return {
      build: {
        rollupOptions: {
          plugins: [
            AnotherPlugin({
              outDir: conf.build.outDir,
            })
          ]
        }
      }
    };

However, conf.build.outDir variable is only defined if the user has provided that option in their own configuration file. If not, it will only be accessible on the configResolved hook, which makes our use case useless.

With this PR, we can make a graceful fallback:

config: (conf) => {
    return {
      build: {
        rollupOptions: {
          plugins: [
            AnotherPlugin({
              outDir: conf.build.outDir ?? configDefaults.build.outDir,
            })
          ]
        }
      }
    };

@ferferga ferferga force-pushed the exported-default-config branch 3 times, most recently from fb7633f to 148cc28 Compare December 18, 2024 20:45
@ferferga ferferga force-pushed the exported-default-config branch from 148cc28 to 56b69bc Compare December 18, 2024 21:02
@ferferga
Copy link
Author

ferferga commented Dec 18, 2024

I have no idea what's wrong with the CJS build, I don't see where side effects are being introduced that pkg is required... 😅

@sapphi-red sapphi-red changed the title refactor: export default config feat: export default config Dec 24, 2024
@sapphi-red
Copy link
Member

Recently we considered exposing the default config values, but didn't do it, because we were not sure how to expose them in a intuitive way.
#18550 (comment)
We need to solve this to expose the default values. Any suggestions are welcome 🙂.

@ferferga
Copy link
Author

@sapphi-red (if you want me to continue the discussion in the original place, please let me know)
What do you think it's exactly "unintuitive" with exposing the defuault object? In my opinion, it's as straightforward as it gets. If I want to just get a value of the default config (like in my posted example, it's much simpler (and faster) to do configDefaults.build.outDir instead of mergeWithDefaults({}).build.outDir.

Also, as an FYI, after diving into Vite's source code, I wanted (for another PR) to change the way the defaults are accessed: I believe they should be factory functions instead, so we always get copies of them and utils like deepClone will not be needed.

@sapphi-red
Copy link
Member

What do you think it's exactly "unintuitive" with exposing the defuault object?

build.outDir is one of the non-problematic ones. The problematic ones are, for example, build.cssMinify, resolve.conditions:

  • build.cssMinify: defaults to 'esbuild' for SSR and build.minify for non-SSR builds
  • resolve.conditions: defaults to DEFAULT_CLIENT_CONDITIONS for non-SSR and ssr.target: 'webworker', otherwise DEFAULT_SERVER_CONDITIONS

What should be set for these options?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants