-
Notifications
You must be signed in to change notification settings - Fork 59
[#410] Fix for qr-code-scanner issue #417
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
This file was deleted.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,8 +1,8 @@ | ||
| "use client"; | ||
|
|
||
| import { useRef, useState } from "react"; | ||
| import { Scanner } from "@yudiel/react-qr-scanner"; | ||
| import { AwardIcon, WrenchIcon } from "lucide-react"; | ||
| import { QrReader } from "react-qr-reader"; | ||
| import { z } from "zod"; | ||
|
|
||
| import type { HackerClass } from "@forge/db/schemas/knight-hacks"; | ||
|
|
@@ -234,15 +234,17 @@ const ScannerPopUp = ({ eventType }: { eventType: "Member" | "Hacker" }) => { | |
| <DialogTitle className="absolute">Check-in {eventType}</DialogTitle> | ||
| </DialogHeader> | ||
| <div> | ||
| <QrReader | ||
| <Scanner | ||
| scanDelay={2000} | ||
| constraints={{ facingMode: "environment" }} | ||
| onResult={async (result, _) => { | ||
| formats={["qr_code"]} | ||
|
Comment on lines
+237
to
+240
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧩 Analysis chain🌐 Web query:
💡 Result: In @yudiel/react-qr-scanner v2.5.1, the
Example: Sources: Citations: 🏁 Script executed: find . -name "scanner.tsx" | grep -E "apps/blade"Repository: KnightHacks/forge Length of output: 176 🏁 Script executed: cat -n ./apps/blade/src/app/_components/shared/scanner.tsx | sed -n '230,260p'Repository: KnightHacks/forge Length of output: 1527 🏁 Script executed: cat -n ./apps/blade/src/app/_components/shared/scanner.tsx | head -20Repository: KnightHacks/forge Length of output: 707 🏁 Script executed: cat -n ./apps/blade/src/app/_components/shared/scanner.tsx | sed -n '240,275p'Repository: KnightHacks/forge Length of output: 1656 Approve library migration, but add defensive validation for QR code format. The const userId = result.rawValue?.substring(5) ?? "";
// or with explicit check:
if (!result.rawValue || result.rawValue.length < 5) {
toast.error("Invalid QR code format");
return;
}
const userId = result.rawValue.substring(5);This prevents runtime errors if the QR code format differs from expectations (e.g., missing prefix or malformed data). 🤖 Prompt for AI Agents |
||
| onScan={async (detectedCodes) => { | ||
| const result = detectedCodes[0]; | ||
| if (!result) return; | ||
| if (scanningRef.current) return; | ||
| scanningRef.current = true; | ||
| try { | ||
| const userId = result.getText().substring(5); | ||
| const userId = result.rawValue.substring(5); | ||
| form.setValue("userId", userId); | ||
|
Comment on lines
+241
to
248
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Add defensive validation for Line 247 assumes Consider validating the format: 🛡️ Suggested defensive check onScan={async (detectedCodes) => {
const result = detectedCodes[0];
if (!result) return;
+ if (!result.rawValue || result.rawValue.length < 5) {
+ toast.error("Invalid QR code format");
+ return;
+ }
if (scanningRef.current) return;
scanningRef.current = true;
try {
const userId = result.rawValue.substring(5);🤖 Prompt for AI Agents |
||
| const eventId = form.getValues("eventId"); | ||
| if (eventId) { | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove the old
react-qr-readerdependency.The new
@yudiel/react-qr-scanneris added (line 40), butreact-qr-reader(line 56) is still listed. Since the migration replaces the old library, it should be removed to reduce bundle size and avoid confusion.🧹 Suggested fix
"@yudiel/react-qr-scanner": "^2.5.1", "canvas-confetti": "^1.9.4", ... "react-qr-code": "^2.0.18", - "react-qr-reader": "3.0.0-beta-1", "recharts": "^3.7.0",Also applies to: 56-56
🤖 Prompt for AI Agents
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
good point rabbit
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@myr124, glad it helps! Sincereact-qr-readeris still listed inpackage.json, here's the one-line fix needed:- "react-qr-reader": "3.0.0-beta-1",Also make sure to run
pnpm installafter the change to update the lockfile and fully remove the old package fromnode_modules. This keeps the bundle lean and avoids any accidental import of the legacy library in the future.Would you like me to plan that change for you?