Skip to content

chore: update vite-plus, switch to bun, drop deepmerge#1458

Merged
fadi-george merged 6 commits into
mainfrom
fg/update-vp
May 8, 2026
Merged

chore: update vite-plus, switch to bun, drop deepmerge#1458
fadi-george merged 6 commits into
mainfrom
fg/update-vp

Conversation

@fadi-george

Copy link
Copy Markdown
Contributor

Description

1 Line Summary

Upgrade vite-plus to 0.1.20, switch package manager from pnpm to bun, and replace the deepmerge package with a small in-repo helper.

Details

  • Bumps vite-plus to 0.1.20 and updates related overrides + dev deps.
  • Migrates the lockfile from pnpm-lock.yaml to bun.lock; .nvmrc is replaced by .node-version.
  • Refreshes AGENTS.md to match the current vite-plus guidance.
  • Adds a deepMerge helper in __test__/support/helpers/general.ts and removes the deepmerge dependency. The helper covers the two existing callers (mockOSMinifiedNotificationPayload, TestContext.getFakeServerAppConfig) with the same plain-object recursion + array-concat behavior we were relying on.

Systems Affected

  • WebSDK
  • Backend
  • Dashboard

Validation

Tests

Info

vp check passes (one pre-existing lint warning in src/shared/config/version.ts unrelated to this change). vp test passes the affected suites that exercise the helper changes (ServiceWorker.test.ts, app.test.ts, 40 tests).

Checklist

  • All the automated tests pass or I explained why that is not possible
  • I have personally tested this on my machine or explained why that is not possible
  • I have included test coverage for these changes or explained why they are not needed

Programming Checklist
Interfaces:

  • Don't use default export
  • New interfaces are in model files

Functions:

  • Don't use default export
  • All function signatures have return types
  • Helpers should not access any data but rather be given the data to operate on.

Typescript:

  • No Typescript warnings
  • Avoid silencing null/undefined warnings with the exclamation point

Other:

  • Iteration: refrain from using `elem of array` syntax. Prefer `forEach` or use `map`
  • Avoid using global OneSignal accessor for `context` if possible. Instead, we can pass it to function/constructor so that we don't call `OneSignal.context`

Screenshots

Info

N/A — tooling and test-helper change only.

Checklist

  • I have included screenshots/recordings of the intended results or explained why they are not needed

Related Tickets

Made with Cursor

@claude claude Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The code change is small and self-contained, but I noticed still reference while this PR deletes in favor of — worth confirming the action falls back to (or updating the workflow paths) before merging.

Extended reasoning...

Overview

This PR is dev-tooling-focused: bumps vite-plus 0.1.13 → 0.1.20, switches the package manager from pnpm to bun (replacing pnpm-lock.yaml with bun.lock), bumps Node 22.22.0 → 24.15.0 (renaming .nvmrc to .node-version), bumps TypeScript 5.9.3 → 6.0.3, vite-plugin-mkcert 1 → 2, and replaces the deepmerge npm package with a small in-repo deepMerge helper used by two test fixtures.

Security risks

None. Changes are confined to dev tooling and test helpers — no production runtime code is modified. The new deepMerge helper uses Object.getPrototypeOf(value) === Object.prototype to detect plain objects, which guards against prototype-pollution-style merging.

Level of scrutiny

Higher than a typical chore. Although the diff is mostly lockfile churn, it bundles several non-trivial changes: a TypeScript major-version bump (5 → 6), a Node major bump (22 → 24), a package-manager switch, and a major bump of vite-plugin-mkcert. Any one of these can surface subtle build/CI issues.

Other factors

The most concrete concern: .github/workflows/ci.yml (line 18), .github/workflows/cd.yml (line 46), and .github/workflows/create-release-pr.yml (line 23) all pass node-version-file: '.nvmrc' to voidzero-dev/setup-vp@v1. This PR deletes .nvmrc. If the action does not silently fall back to .node-version, CI/CD will break post-merge. This is the kind of thing that's safest verified by a human (either by trusting the action's behavior or updating the workflow paths in the same PR).

The deepMerge helper itself is correct for the two callers — plain-object recursion + array concat matches the prior deepmerge package's defaults. One minor behavior delta: explicit undefined values in the source are skipped (vs. deepmerge which would overwrite), but since both call-sites pass PartialDeep/RecursivePartial test data, this is unlikely to matter in practice.

Comment thread .nvmrc Outdated
Comment thread package.json Outdated
@fadi-george fadi-george requested a review from sherwinski May 7, 2026 22:34
@fadi-george fadi-george merged commit 75732e9 into main May 8, 2026
3 checks passed
@fadi-george fadi-george deleted the fg/update-vp branch May 8, 2026 01:20
sherwinski added a commit that referenced this pull request May 19, 2026
The .nvmrc file was removed in #1458 when the project switched to bun,
but cd.yml and create-release-pr.yml still reference it via
node-version-file, causing setup-vp to fail with
"node-version-file not found". Mirror the fix already applied in ci.yml.
@github-actions github-actions Bot mentioned this pull request May 19, 2026
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