Skip to content

chore: Remove no-op function in plugin config logic#8501

Merged
sheremet-va merged 2 commits intovitest-dev:mainfrom
TrevorBurnham:remove-undefined-values
Apr 9, 2026
Merged

chore: Remove no-op function in plugin config logic#8501
sheremet-va merged 2 commits intovitest-dev:mainfrom
TrevorBurnham:remove-undefined-values

Conversation

@TrevorBurnham
Copy link
Copy Markdown
Contributor

Description

Per previous discussion at #8487, this PR removes the removeUndefinedValues utility function added by #2281, which had a bug that made it effectively a no-op. The issue reported at #2153 somehow hasn't resurfaced in 3 years.

I've also removed the ?? {} after viteConfig.test: deepMerge treats undefined values and empty objects as equivalent, so there's no reason to allocate an extra object here.

Please don't delete this checklist! Before submitting the PR, please make sure you do the following:

  • It's really useful if your PR references an issue where it is discussed ahead of time. If the feature is substantial or introduces breaking changes without a discussion, PR might be closed.
  • Ideally, include a test that fails without this PR but passes with it. (Author note: Unfortunately, there's no existing test coverage for this part of the codebase.)
  • Please, don't make changes to pnpm-lock.yaml unless you introduce a new test example.
  • Please check Allow edits by maintainers to make review process faster. Note that this option is not available for repositories that are owned by Github organizations.

Documentation

  • If you introduce new functionality, document it. You can run documentation with pnpm run docs command.

Changesets

  • Changes in changelog are generated from PR name. Please, make sure that it explains your changes in an understandable manner. Please, prefix changeset messages with feat:, fix:, perf:, docs:, or chore:.

@netlify
Copy link
Copy Markdown

netlify Bot commented Aug 28, 2025

Deploy Preview for vitest-dev ready!

Name Link
🔨 Latest commit 0aa8c71
🔍 Latest deploy log https://app.netlify.com/projects/vitest-dev/deploys/69d565eb04dd180009d925bc
😎 Deploy Preview https://deploy-preview-8501--vitest-dev.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

@sheremet-va
Copy link
Copy Markdown
Member

Seems like it does do something (CI is failing)

@TrevorBurnham TrevorBurnham force-pushed the remove-undefined-values branch from 586877d to 4bce2e6 Compare September 14, 2025 20:00
@TrevorBurnham
Copy link
Copy Markdown
Contributor Author

Seems like it does do something (CI is failing)

Weird… I thought the CI failures were random, but I rebased and see the same failures. And they're consistent across all OSes the CI runs against:

@vitest/test-config : test test/config
  test/config test$ vitest --typecheck.enabled
   ELIFECYCLE  Command failed with exit code 1.

In local dev, if I run the test/config tests (cd test/config && pnpm test), all tests pass. I get failures on my machine when I run pnpm run test:ci, but they're unrelated failures (random timeouts in istanbul and `istanbul-browser).

Any idea why that error might be happening in CI? It's weird that there isn't even any output before the process exits, right? 😕

@TrevorBurnham TrevorBurnham force-pushed the remove-undefined-values branch 2 times, most recently from 4673953 to 276d950 Compare September 28, 2025 19:41
@TrevorBurnham
Copy link
Copy Markdown
Contributor Author

I've confirmed that the same CI failures occur when running the CI workflow against main: https://github.com/TrevorBurnham/vitest/actions/runs/18079109566/job/51439858698

@TrevorBurnham TrevorBurnham force-pushed the remove-undefined-values branch from 276d950 to a0f6a4c Compare November 27, 2025 17:05
@sheremet-va sheremet-va merged commit 82c858d into vitest-dev:main Apr 9, 2026
15 of 16 checks passed
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