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

refactor: optimizeDeps back to top level #18465

Merged
merged 2 commits into from
Oct 28, 2024

Conversation

patak-dev
Copy link
Member

@patak-dev patak-dev commented Oct 25, 2024

Description

In the current Environment API design, we made every top-level option act as a default for all environments. To make that happen in a backward compatible way, and because deps optimization is dev only, we moved optimizeDeps (only applies to client) to dev.optimizeDeps (default for all environments).

Even if consistency is improved, this also brings issues. In https://github.com/vitejs/vite/pull/18397/files, we had to use:

  environments: {
    client: {
      dev: {
        optimizeDeps: {
          include: [ 'gsap' ],
        },
      },
    },
  },

instead of

  optimizeDeps: {
    include: [ 'gsap' ],
  },

to avoid gsap to also be optimized during SSR. Apart from the three extra levels of nesting environments.client.dev., this is bad because we now need to explain what environments are for users that are building a simple SPA/MPA. The config for them, should still works configuring the whole app at top level.

This triggered @bluwy into exploring some design changes to the Environment API: #18415. We explored reducing the needed nesting and exposing environments with #18439. But the design gets more complex and the user still needs to understand environments in the case above even if using a top level client.

This PR starts some changes discussed in this thread. The idea is that we are going to change the concept of top-level props from being "defaults" to "app configuration". Environments will in general inherit from these options, but some top-level options will only affect the client because they don't make sense as a general default for all environments (optimizeDeps is a good example, also dev.warmup will probably be client only). This means that a user building a SPA/MPA can configure their app (that has a single environment, the client) using only top-level keys. And when other environments are added later (if SSR is introduced), it can configure them with environments.ssr. But until then, there is no need to know what an environment is. environments.client will not get much use in general after this PR.

This change will also avoid needing to ask users to move from optimizeDeps, that would have resulted in a lot of config changes and churn.

@patak-dev patak-dev requested a review from bluwy October 25, 2024 14:02
@patak-dev patak-dev added the p3-significant High priority enhancement (priority) label Oct 25, 2024
@patak-dev patak-dev added this to the 6.0 milestone Oct 25, 2024
@patak-dev patak-dev added the feat: environment API Vite Environment API label Oct 25, 2024
Copy link
Member

@bluwy bluwy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm probably a bit biased to be approving, but taking that hat off, I do think moving optimizeDeps back help with migration at the meantime where it helps with the mapping of existing config.optimizeDeps and config.ssr.optimizeDeps.

@sheremet-va
Copy link
Member

Honestly, I thought it already works like this 🫣

Copy link
Collaborator

@hi-ogawa hi-ogawa left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was also my expectation as well.

Copy link
Member

@sapphi-red sapphi-red left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thinking about it back and forth, I think this fits more to the current way of configuring Vite.

@patak-dev
Copy link
Member Author

Ok, let's go with this approach then

@patak-dev patak-dev merged commit 1ac22de into main Oct 28, 2024
15 checks passed
@patak-dev patak-dev deleted the refactor/optimize-deps-back-to-top-level branch October 28, 2024 08:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feat: environment API Vite Environment API p3-significant High priority enhancement (priority)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants