Skip to content

fix: less promises for scanning and await with allSettled#11245

Merged
patak-cat merged 2 commits intovitejs:mainfrom
dominikg:refactor/scan-promises
Dec 7, 2022
Merged

fix: less promises for scanning and await with allSettled#11245
patak-cat merged 2 commits intovitejs:mainfrom
dominikg:refactor/scan-promises

Conversation

@dominikg
Copy link
Contributor

@dominikg dominikg commented Dec 7, 2022

alternative to #11243

doesn't add cancel() function to as that may have performance implications and the way the function has to be passed around isn't nice.

Description

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.

@dominikg dominikg changed the title fix: promises with allSettled fix: less promises for scanning and await with allSettled Dec 7, 2022
@dominikg dominikg mentioned this pull request Dec 7, 2022
Closed
9 tasks
patak-cat
patak-cat approved these changes Dec 7, 2022
View reviewed changes
Copy link
Member

@patak-cat patak-cat left a comment

Choose a reason for hiding this comment

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

I think we should go with this one for v4. Let's check the canceling with more time, and actually, see how much we win perf wise for the extra complexity.

@patak-cat
Copy link
Member

patak-cat commented Dec 7, 2022

/ecosystem-ci run

@vite-ecosystem-ci
Copy link

vite-ecosystem-ci bot commented Dec 7, 2022 •
edited
Loading

📝 Ran ecosystem CI: Open

suite result
astro ✅ success
histoire ✅ success
iles ✅ success
ladle ✅ success
laravel ✅ success
marko ✅ success
nuxt-framework ❌ failure
rakkas ⏹️ cancelled
sveltekit ❌ failure
vite-plugin-ssr ❌ failure
vite-plugin-react ✅ success
vite-plugin-react-swc ✅ success
vite-plugin-svelte ✅ success
vite-plugin-vue ✅ success
vite-setup-catalogue ✅ success
vitepress ✅ success
vitest ❌ failure
windicss ✅ success

@patak-cat
Copy link
Member

patak-cat commented Dec 7, 2022

/ecosystem-ci run sveltekit

@vite-ecosystem-ci
Copy link

vite-ecosystem-ci bot commented Dec 7, 2022 •
edited
Loading

📝 Ran ecosystem CI: Open

suite result
sveltekit ✅ success

@patak-cat patak-cat changed the title fix: less promises for scanning and await with allSettled fix: less promises for scanning and await with allSettled Dec 7, 2022
@patak-cat patak-cat merged commit 45b170e into vitejs:main Dec 7, 2022
@sapphi-red sapphi-red mentioned this pull request Dec 8, 2022
Merged
9 tasks
futurGH pushed a commit to futurGH/vite that referenced this pull request Feb 26, 2023
@dominikg @futurGH
fix: less promises for scanning and await with allSettled (vitejs#11245)
e0a2b50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@patak-cat patak-cat patak-cat approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@dominikg @patak-cat