Skip to content

fix: unbreak main — Navbar useEffect, duplicate stepRef, and the SkeletonBar prop typing that reds every deploy preview - #492

Open
jarik2014 wants to merge 3 commits into
SmartDropLabs:mainfrom
jarik2014:fix/navbar-useeffect
Open

jarik2014 wants to merge 3 commits into
SmartDropLabs:mainfrom
jarik2014:fix/navbar-useeffect

Conversation

@jarik2014

@jarik2014 jarik2014 commented Sep 24, 2026 •

Copy link
Copy Markdown

main does not build. Four separate defects, each of which takes the app down on its own. This PR fixes all four in one branch, because the point is a green main, not four separate red things.

1. src/components/Navbar/Navbar.tsx — useEffect was never imported

src/components/Navbar/Navbar.tsx(206,3): error TS2304: Cannot find name 'useEffect'.

The app crashes on load, and eleven Navbar unit tests were red on main. They pass with the import restored (11/11).

2. src/hooks/useLockFlow.ts — stepRef declared twice

A merge artefact left two const stepRef declarations in the same scope, so the module did not even parse (Module parse failed).

3. src/app/loading.tsx — SkeletonBar's w prop is narrower than its call sites

src/app/loading.tsx(42,11): error TS2322: Type '{ base: string; md: string; }' is not assignable to type 'string'.

The farm skeleton passes responsive Chakra values at six call sites, while the Flex in the same component already takes a responsive direction the same way — only the prop type was wrong. next build fails on it, which is why every Netlify deploy preview in this repository comes back red, whichever branch triggered it. The type now comes from Chakra (BoxProps["w"]) rather than a hand-written union.

4. src/i18n.ts and src/request.ts — locales.includes(locale as any)

next build runs ESLint, and two @typescript-eslint/no-explicit-any errors stopped it there. The cast silenced the checker on a value that genuinely arrives from outside the process (the request), which is precisely the case the fallback exists for. Both now go through an isLocale() type predicate, so the narrowing is real.

Verification

$ pnpm exec tsc --noEmit      # clean — the four errors above are the entire output on main
$ pnpm build                  # succeeds; route table printed, exit 0

Plus, on the sibling branches: Navbar unit tests 11/11, farm e2e 5/5, visual-regression e2e 3/3.

Nothing was disabled, skipped or weakened to get here — no eslint-disable, no @ts-ignore, no narrowed test globs. The four fixes are the whole diff.

Two things I left alone on purpose, both out of scope: src/request.ts is imported by nothing (near-duplicate of i18n.ts, dead code — a separate change), and the build prints unused-variable warnings in useLockFlow.ts, src/lib/*, and src/lib/soroban.ts that predate this branch and do not fail the build.

@netlify

netlify Bot commented Sep 24, 2026 •

Copy link
Copy Markdown

✅ Deploy Preview for smart-drop ready!

Name Link
🔨 Latest commit 0db5683
🔍 Latest deploy log https://app.netlify.com/projects/smart-drop/deploys/6ab6c5af52e0400008de4227
😎 Deploy Preview https://deploy-preview-492--smart-drop.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.

@netlify

netlify Bot commented Sep 24, 2026 •

Copy link
Copy Markdown

✅ Deploy Preview for spiffy-melomakarona-eb1e8a ready!

Name Link
🔨 Latest commit 0db5683
🔍 Latest deploy log https://app.netlify.com/projects/spiffy-melomakarona-eb1e8a/deploys/6ab6c5af5aec0600086b6249
😎 Deploy Preview https://deploy-preview-492--spiffy-melomakarona-eb1e8a.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.

@jarik2014 jarik2014 changed the title fix: import useEffect in Navbar — the app crashes on main without it fix: unbreak main — missing useEffect import, duplicate stepRef Sep 24, 2026
@jarik2014

Copy link
Copy Markdown
Author

Confirmation that these two fixes unblock the E2E suite, not just the build:

$ pnpm exec playwright test --config e2e/playwright.config.ts e2e/visual-regression.spec.ts
  3 passed (36.4s)

$ pnpm exec playwright test --config e2e/playwright.config.ts e2e/farm.spec.ts
  5 passed (45.9s)

Before: farm.spec.ts failed 5 of 5 with a build-error overlay on /farm and visual-regression.spec.ts could not start at all (web server never became ready). After: 8 of 8 pass, including the screenshot comparisons against the committed baselines.

The third breakage is still there and still blocks a production build: ./src/app/loading.tsx:42 passes a responsive w={{ base: "100%", md: "120px" }} to SkeletonBar, whose w prop is typed string. Say the word on which side you want changed and I will do it.

@jarik2014

Copy link
Copy Markdown
Author

One more piece of evidence, from the checks on this PR: every status here is a Netlify deploy preview, and all of them fail — Header rules, Pages changed, Redirect rules, deploy-preview, on both sites. They fail because the production build never finishes, which is the loading.tsx / SkeletonBar type error from the section above, not anything in this diff. There are no unit-test jobs on this repo's checks at all, so Netlify is the whole signal a reviewer sees — and it will stay red on every PR until that third breakage is fixed. I am not touching it without your call on which side should change, but it is the difference between a green and a red preview for all four of our PRs.

@jarik2014 jarik2014 changed the title fix: unbreak main — missing useEffect import, duplicate stepRef fix: unbreak main — Navbar useEffect, duplicate stepRef, and the SkeletonBar prop typing that reds every deploy preview Sep 24, 2026
@jarik2014

Copy link
Copy Markdown
Author

Ran the real build locally on the branch, since this is what the deploy previews run:\n\n\n$ pnpm exec tsc --noEmit\n(no output — clean)\n\n$ pnpm build\n✓ Compiled successfully\n...route table...\nƒ Middleware 34 kB\nexit 0\n\n\nSo the four fixes together are what main needs to build again — the previews that have been red on every branch should go green with this merge. The SkeletonBar one (fix 3) is the reason they were red on branches that had nothing to do with the farm skeleton.

@jarik2014

Copy link
Copy Markdown
Author

Added one more commit: a lockfile sync that is not about this PR's own subject, but is the reason every Netlify check on this PR was red.

package.json has carried next-intl: ^4.14.6 while pnpm-lock.yaml never got it. Netlify installs with --frozen-lockfile, so the install fails before the build is reached:

ERR_PNPM_OUTDATED_LOCKFILE  Cannot install with "frozen-lockfile" because
pnpm-lock.yaml is not up to date with <ROOT>/package.json
    Failure reason: specifiers in the lockfile (...) don't match specs in package.json (...)
    the only difference is next-intl

That is why the six checks on this PR ("Header rules", "Pages changed", "Redirect rules" — the two deploy previews each report three) were failing: they report the preview failing to build, and the preview cannot build while the install is refused. It is repo-wide, not specific to this branch.

The commit adds next-intl and its dependency tree to the lockfile and nothing else — pnpm install --no-frozen-lockfile reports no specifier changes for existing packages, the 439 added lines are all entries the package needs (icu-minify, intl-messageformat, negotiator, po-parser, the SWC extractor plugin). Verified after the change:

$ pnpm install --frozen-lockfile
Already up to date          # the exact command that used to abort

$ pnpm build
…compiled successfully, full route table printed, middleware 34 kB

The dependency is real, not stale: src/i18n.ts and src/request.ts both import { getRequestConfig } from "next-intl/server".

This commit is also on the other three open PRs from the same base (#490, #491, #493) so their previews can go green independently of this one merging.

@jarik2014

Copy link
Copy Markdown
Author

this is the PR that unblocks #490, #491 and #493 — they are all mergeable but red because main is broken. could you take this one first?

`pnpm build` fails before it compiles anything:

  ./src/hooks/useLockFlow.ts
  Module parse failed: Identifier 'stepRef' has already been declared (31:10)

The hook declares it twice -- once in the shared "use refs to avoid recreating
execute" block, and again six lines later under the SmartDropLabs#396 comment:

  51  const stepRef = useRef(step);
  52  stepRef.current = step;
  ...
  61  const stepRef = useRef(step);   <- the duplicate, with the rationale above it
  62  stepRef.current = step;

Same merge window as the missing `useEffect` import in the previous commit, and the
same effect: `/farm` cannot compile, so the dev server serves a build-error overlay
and its E2E specs time out looking for a Connect button that never renders.

Kept the second declaration, because the SmartDropLabs#396 comment explaining why the ref exists
belongs next to it, and deleted the first pair. The comment above the surviving
`walletApiRef` said "when step or walletApi change"; with `stepRef` no longer there it
says what it now means.

Verification: the module parses again -- `pnpm build` moves past this file, and
`grep -c "const stepRef" src/hooks/useLockFlow.ts` is 1 from 2.
next build runs ESLint, and two no-explicit-any errors stopped it: both
i18n.ts and request.ts guarded their locale with
`locales.includes(locale as any)`. The cast silenced the checker on a
value that genuinely arrives from outside the process (the request), which
is exactly the case the fallback exists for.

Both now go through an isLocale() type predicate in i18n.ts, so the
narrowing is real and the fallback keeps its meaning. The two files are
otherwise near-identical duplicates; request.ts is imported by nothing,
but removing dead code is a separate change.
@jarik2014

Copy link
Copy Markdown
Author

Rebased onto current main (5d058e5) and force-pushed. The branch is now three commits; two of the original five were dropped because main already contains them:

  • fix: import useEffect in Navbar — already on main; git skipped the commit during the rebase.
  • fix: widen SkeletonBar's width prop — main fixed the same file its own way (w?: string | { base: string; md: string } plus type BoxProps in the Chakra import), so I kept upstream's version and that commit became empty.

Conflict that did need resolving, src/hooks/useLockFlow.ts: upstream removed the duplicate stepRef but left walletApiRef declared twice in the same block (const walletApiRef at both line 51 and 63 of the merged attempt — the same class of mistake as the one this PR was opened for). Resolved by keeping exactly one declaration of each ref — walletApiRef with upstream's wording of the comment, then stepRef with its #396 comment — so the module parses again. grep -c "const walletApiRef =" and grep -c "const stepRef =" are both 1.

What remains in the diff (4 files, +451/−57): the lockfile sync, the locale guard in src/i18n.ts/src/request.ts, and the useLockFlow de-duplication.

Verification, run on this branch:

$ pnpm install --frozen-lockfile
Lockfile is up to date, resolution step is skipped
Already up to date
Done in 1.1s using pnpm v9.15.9        # exit 0

The same command on current main fails, which is what this PR is for:

$ pnpm install --frozen-lockfile      # in a worktree of origin/main
 ERR_PNPM_OUTDATED_LOCKFILE  Cannot install with "frozen-lockfile" because pnpm-lock.yaml is not up to date with package.json
 specifiers in the lockfile (... no "next-intl" ...) don't match specs in package.json (... "next-intl":"^4.14.6" ...)
$ pnpm typecheck
> tsc --noEmit        # exit 0

$ pnpm build
> node scripts/check-css.mjs && next build
  ... route table printed, /farm 14.5 kB, First Load JS 103 kB, exit 0

So the build and the frozen install both pass here while main still cannot install. Intent of the PR is unchanged; the lockfile is the only large part of the diff and it is regenerated, not hand-edited.

This branch has not been deployed

No deployments
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.

1 participant