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: add async support for vite config file #2758

Merged
merged 1 commit into from
Mar 31, 2021
Merged

Conversation

xiaoxiangmoe
Copy link
Contributor

Description

feat: add async support for vite config file

for example:
If the config needs to call async function, it can export a async function instead:

export default async ({ command, mode }) => {
  const data = await asyncFunction();
  return {
    // build specific config
  } 
}

Additional context


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.

@patak-dev
Copy link
Member

patak-dev commented Mar 29, 2021

This looks good. How would it work when using defineConfig ? Do we need a defineAsyncConfig utility?

Edit: It could be used like

import { defineConfig } from 'vite'

export default async ({ command, mode }) => {
  const data = await asyncFunction();
  return defineConfig({
    // build specific config
  })
}

but it would be nice to have

import { defineAsyncConfig } from 'vite'

export default defineAsyncConfig(async { command, mode }) => {
  const data = await asyncFunction();
  return {
    // build specific config
  }
})

That also type checks command and mode. What do you think? I think it is ok if you prefer to discuss this in another PR.

@Shinigami92
Copy link
Member

I think export default defineConfig(async ({ command, mode }) => { // ... should be just enough
And then handle it correctly in the defineConfig dependently

The really nice thing behind defineConfig should be that it is just usable/addable as easy as possible

- export default config
+ export default defineConfig(config

- export default () => 
+ export default defineConfig(() =>

- export default async () => 
+ export default defineConfig(async () =>

@Shinigami92
Copy link
Member

There are two smart ways of handle this: https://stackoverflow.com/questions/27746304/how-do-i-tell-if-an-object-is-a-promise

First would be to check if it's a promise, or second (and I think this is really smart ^^) just use Promise.resolve(configFn).then

@xiaoxiangmoe
Copy link
Contributor Author

@Shinigami92 We can just use await

@Shinigami92
Copy link
Member

Seems it's not my day 😆 and I'm thinking far to complicated

@xiaoxiangmoe
Copy link
Contributor Author

@yyx990803 Anything else should I do?

@Shinigami92 Shinigami92 added the p2-nice-to-have Not breaking anything but nice to have (priority) label Mar 30, 2021
@patak-dev
Copy link
Member

patak-dev commented Mar 30, 2021

@xiaoxiangmoe if you would like to modify defineConfig in a separate PR (or prefer that others would do it), then there is nothing to do for this one. But I think we should have that modification before merging this PR for consistency.

Edit: Ok, IMO we can merge this as is but it would be good to have that modification soon. We can use it like this until that time:

import { defineConfig } from 'vite'

export default async ({ command, mode }) => {
  const data = await asyncFunction();
  return defineConfig({
    // build specific config
  })
}

Note: Evan needs would need to approve this one.

@Shinigami92
Copy link
Member

@patak-js I'm not sure if you're messing something up, but I would thing it would be this way 🤔:

import { defineConfig } from 'vite'

export default defineConfig(async ({ command, mode }) => {
  const data = await asyncFunction();
  return {
    // build specific config
  }
}

@patak-dev
Copy link
Member

@Shinigami92 updated my wording, the code snippet was referring to how it can be used until we modify defineConfig to support async functions.

@xiaoxiangmoe
Copy link
Contributor Author

Note: Evan needs would need to approve this one.

@yyx990803 Can you review it?

@patak-dev
Copy link
Member

Hey @xiaoxiangmoe, we are using GitHub's review requests when we need Evan's input so he can look at all the issues and PR that require his attention when he is focusing on Vite. There is no need to explicitly ping him with an extra message.

@xiaoxiangmoe
Copy link
Contributor Author

Oh, I know. Sorry.
GitHub's review requests is great.

@antfu antfu merged commit aee8b37 into vitejs:main Mar 31, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
p2-nice-to-have Not breaking anything but nice to have (priority)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants