Skip to content

Target Node 22 LTS; bump axios to 1.19 and yup to 1.7 - #137

Open
devin-ai-integration[bot] wants to merge 1 commit into
developfrom
devin/1786532389-node22-axios-yup
Open

devin-ai-integration[bot] wants to merge 1 commit into
developfrom
devin/1786532389-node22-axios-yup

Conversation

@devin-ai-integration

@devin-ai-integration devin-ai-integration Bot commented Aug 12, 2026

Copy link
Copy Markdown

Summary

Runtime standardization plus the two low-risk dependency majors. No framework migrations (React Router / XState / MUI / Express untouched), still Yarn Classic.

  • engines.node: ^20.0.0 || ^22.0.0^22.0.0; .node-version and .nvmrc pinned to 22.20.0 to match the CI image cypress/browsers:22.20.0 (workflow already on 22.20.0, unchanged). @types/node moved to ^22 to match the runtime line.

  • axios 0.28.11.19.0. Only impacted call site is the request interceptor, where v1 passes an AxiosHeaders instance instead of a plain object:

    - // @ts-ignore
    - config.headers["Authorization"] = `Bearer ${accessToken}`;
    + config.headers.set("Authorization", `Bearer ${accessToken}`);

    All other usages (httpClient.get/post/patch with params, resp.data) are unchanged between 0.x and 1.x.

  • yup 0.32.111.7.1, and dropped @types/yup (v1 ships its own types). The six formik schemas (SignInForm, SignUpForm, BankAccountForm, CommentForm, UserSettingsForm, TransactionCreateStepTwo) already use the v1-compatible API (mixed<T>(), ref(), oneOf) and needed no changes; I diffed v0.32 vs v1 validation output for the schemas whose messages Cypress asserts on (number().required("Please enter a valid amount") etc.) and the error strings are identical, so cypress/tests/ui/new-transaction.spec.ts assertions still hold.

postinstall (husky install && patch-package) succeeds and the only patch (react-virtualized+9.22.5.patch) still applies — that package wasn't bumped.

Validated locally on Node 22.20.0: yarn types, yarn lint, yarn test:unit:ci (44 passed), yarn build:ci all pass.

Link to Devin session: https://app.devin.ai/sessions/7ed3e010154744e6a74ada236b7cd2b6
Requested by: @KPH-coder


Devin Review

Status Commit
⚪ Not started

Run Devin Review

Open in Devin Review (Staging)
Open in Devin Review

Co-Authored-By: Kalle Harnos <kalleharnos@gmail.com>
@devin-ai-integration

Copy link
Copy Markdown
Author

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

  • Address comments on this PR. Add '(aside)' to your comment to have me ignore it.
  • Look at CI failures and help fix them

Note: I can only respond to comments from users who have write access to this repository.

⚙️ Control Options:

  • Disable automatic comment, CI, and merge conflict monitoring

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Devin Review found 3 potential issues.

Open in Devin Review

Comment thread src/utils/asyncUtils.ts
const accessToken = localStorage.getItem(process.env.VITE_AUTH_TOKEN_NAME!);
// @ts-ignore
config.headers["Authorization"] = `Bearer ${accessToken}`;
config.headers.set("Authorization", `Bearer ${accessToken}`);

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

📝 Info: Interceptor header mutation is valid for axios 1.x

In axios 1.x, Axios.prototype.request builds config.headers = AxiosHeaders.concat(contextHeaders, headers) before request interceptors execute, so config.headers is guaranteed to be an AxiosHeaders instance here and .set() is safe (it is not a plain object as in 0.x). No other call site in src/machines/*.ts touches headers, so this is the only site needing migration.

Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Comment thread src/utils/asyncUtils.ts
const accessToken = localStorage.getItem(process.env.VITE_AUTH_TOKEN_NAME!);
// @ts-ignore
config.headers["Authorization"] = `Bearer ${accessToken}`;
config.headers.set("Authorization", `Bearer ${accessToken}`);

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

📝 Info: Query params are flat, so axios 1.x serializer change is not observable

axios 1.x replaced the default params serializer (nested objects now use bracket notation instead of 0.x JSON stringification). All params passed by the machines (src/machines/usersMachine.ts:11-13, personalTransactionsMachine.ts:10-12, notificationsMachine.ts, publicTransactionsMachine.ts, contactsTransactionsMachine.ts) are flat string/number maps derived from omit("type", event) and the filter machine (src/machines/transactionFiltersMachine.ts:22-99), so no nested values reach the serializer and backend query parsing is unaffected.

Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Comment thread package.json
"webpack": "5",
"xstate": "4.38.3",
"yup": "0.32.11"
"yup": "1.7.1"

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

🔍 yup v1 upgrade: all six schemas use v1-compatible API with explicit messages

Reviewed every yup consumer (SignInForm, SignUpForm, BankAccountForm, CommentForm, UserSettingsForm, TransactionCreateStepTwo). All validators (string().min/max/length/matches/email, number().required, mixed<T>().oneOf, ref) exist unchanged in v1 and every message asserted by Cypress specs is supplied explicitly, so the removal of @types/yup and the major bump should not change runtime error strings. Worth spot-checking the Cypress UI suite once in CI since default type-error messages (e.g. non-numeric amount input) are the only ones not overridden.

Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

@devin-ai-integration

Copy link
Copy Markdown
Author

E2E verification — axios 1.19 + yup 1.7 + Node 22

Ran the app locally on Node 22.20.0 (yarn dev, freshly seeded db) and exercised every axios/yup-touching flow through the UI. All scenarios passed. No axios/yup/network errors in the browser console (grepped the full console dump for axios|yup|Network Error|ERR_BAD|401|CORS — zero matches; only pre-existing MUI ref + XState deprecation warnings).

yup 1.x validation still renders the right messages

yup validation on new transaction
Exact strings "Please enter a valid amount" and "Please enter a note"; Pay/Request disabled. Also verified "Enter a first name" (User Settings), "Password does not match" (ref() in oneOf, sign-up), and "Username is required" (sign-in).

axios 1.x POST/PATCH + headers.set() interceptor work end-to-end

payment success and balance change
Paid $5.00 → balance $1,509.53 → $1,504.53, transaction visible in Mine.

Feeds, search params, likes/comments, settings PATCH, bank account (GraphQL POST), re-auth
  • Everyone / Friends / Mine feeds all load populated (axios GET with query params).
  • Payee search: typing "Dina" re-orders results so Dina20 is first → params reach the backend.
  • Like 0→1 and comment "nice one" persist after reload.
  • User Settings first name → "AxiosTest" persists after reload (PATCH).
    settings persisted
  • Bank account "Axios Test Bank" created via the GraphQL POST path, persists after reload.
    bank account persisted
  • Sign out → sign in again works, confirming withCredentials cookie auth is intact under axios 1.x.
    re-login feed with persisted data

Tested by Devin — session

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.

0 participants