Skip to content

fix script loading problems#97

Merged
9cb14c1ec0 merged 5 commits intomasterfrom
bugfixtry3
Oct 8, 2025
Merged

fix script loading problems#97
9cb14c1ec0 merged 5 commits intomasterfrom
bugfixtry3

Conversation

@9cb14c1ec0
Copy link
Copy Markdown
Collaborator

@9cb14c1ec0 9cb14c1ec0 commented Oct 7, 2025

Fixes # .

Changes proposed in this pull request

To test (it takes a while): npm install github:<username>/venom#<branch>

Summary by CodeRabbit

  • New Features

    • Built-in ASCII QR generation for terminal display (compact and large modes).
    • Bundled QR codec to generate QR images at runtime.
  • Bug Fixes

    • More reliable asset loading across environments with fallback handling.
    • Improved login status detection for more consistent paired/unpaired recognition.
    • Safer QR generation with better error handling and reporting.
  • Chores

    • Build step added to include QR/vendor assets in distributions.

@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Oct 7, 2025

Walkthrough

Adds an internal QRCode library and ASCII QR renderer, switches auth to use the internal renderer, makes WhatsApp asset loading runtime-agnostic, and updates build scripts to copy vendor assets into dist.

Changes

Cohort / File(s) Summary of changes
Build scripts
package.json
Added copy:vendor script to copy vendor into dist/; updated build:venom to run npm run copy:vendor after build steps.
WhatsApp asset path resolution
src/api/whatsapp.ts
Added cross-platform getDirname() and checkFileExists(); load wapi/middleware from computed dirname if present, otherwise fall back to node_modules/venom-bot/dist/lib/....
Auth QR integration
src/controllers/auth.ts
Replaced external qrcode-terminal with internal generateASCIIQR; asciiQr now awaits generateASCIIQR; simplified UNPAIRED detection to canvas presence; updated async error handling/logging.
QR ASCII renderer utility
src/utils/qr-generator.ts
New generateASCIIQR(input, opts?) that dynamically imports vendor QRCode modules and renders QR as compact (small) or large ASCII with borders/padding.
Vendor QRCode core
vendor/QRCode/index.js
New full QRCode implementation (constructor, encoding, masking, error correction, mapping, rendering) and export.
Vendor QRCode primitives & helpers
vendor/QRCode/*
Added multiple vendor modules: QR8bitByte.js, QRBitBuffer.js, QRErrorCorrectLevel.js, QRMaskPattern.js, QRMath.js, QRMode.js, QRPolynomial.js, QRRSBlock.js, QRUtil.js — QR math, constants, polynomial/bitbuffer, RS blocks, utilities used by QR generator.
Other vendor assets
vendor/QRCode/...
New static mappings/constants and data structures required by the QR implementation (error-correction maps, mask patterns, mode flags).

Sequence Diagram(s)

sequenceDiagram
  autonumber
  actor User
  participant AuthController as Auth Controller
  participant QRUtil as generateASCIIQR
  participant Vendor as Vendor QR Modules

  User->>AuthController: request QR for pairing (code)
  AuthController->>QRUtil: generateASCIIQR(code, opts)
  note right of QRUtil: dynamic import of vendor modules (QRCode, QRUtil, etc.)
  QRUtil->>Vendor: instantiate QRCode(), addData(), make()
  Vendor-->>QRUtil: QR matrix
  QRUtil-->>AuthController: ASCII QR string
  AuthController-->>User: print/return ASCII QR
Loading
sequenceDiagram
  autonumber
  participant WAAPI as src/api/whatsapp.ts
  participant Resolver as getDirname()
  participant FS as filesystem

  WAAPI->>Resolver: compute base dirname (__dirname or import.meta.url)
  Resolver-->>WAAPI: base path
  WAAPI->>FS: checkFileExists(../../lib/wapi/wapi.js)
  alt asset exists in package
    FS-->>WAAPI: true
    WAAPI->>FS: load ../../lib/wapi/wapi.js & ../../lib/middleware/middleware.js
  else fallback
    FS-->>WAAPI: false
    WAAPI->>FS: load node_modules/venom-bot/dist/lib/.../wapi.js & middleware.js
  end
  WAAPI->>WAAPI: page.evaluate(...) to inject/evaluate scripts
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related PRs

  • eslint upgrade #65 — Similar changes to asset-loading logic in src/api/whatsapp.ts (wapi/middleware resolution).
  • Fix qr code #93 — Related modifications to auth QR/unpaired detection and QR generation behavior in src/controllers/auth.ts.

Poem

I nibble code by moonlit light,
Stitched QR squares into ASCII bright.
Vendor bits tucked into dist,
Auth prints codes with a tiny twist.
Hop, build, copy — away I flight. 🐇✨

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The title succinctly highlights the correction of script loading issues, directly reflecting the dynamic path resolution and build script updates made to address loading failures in the changeset. It is concise, clear, and tied to a real aspect of the modifications.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch bugfixtry3

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 3

🧹 Nitpick comments (1)
vendor/QRCode/QRMath.js (1)

31-42: Avoid redeclaring the loop variable 'i'.

The variable i is redeclared in three consecutive for loops. While this works in JavaScript with var, it's better practice to use unique variable names or let for block-scoped declarations to improve code clarity and avoid potential confusion.

Apply this diff to use distinct variable names:

-for (var i = 0; i < 8; i++) {
+for (var i = 0; i < 8; i++) {
 	QRMath.EXP_TABLE[i] = 1 << i;
 }
-for (var i = 8; i < 256; i++) {
+for (var j = 8; j < 256; j++) {
-	QRMath.EXP_TABLE[i] = QRMath.EXP_TABLE[i - 4]
-		^ QRMath.EXP_TABLE[i - 5]
-		^ QRMath.EXP_TABLE[i - 6]
-		^ QRMath.EXP_TABLE[i - 8];
+	QRMath.EXP_TABLE[j] = QRMath.EXP_TABLE[j - 4]
+		^ QRMath.EXP_TABLE[j - 5]
+		^ QRMath.EXP_TABLE[j - 6]
+		^ QRMath.EXP_TABLE[j - 8];
 }
-for (var i = 0; i < 255; i++) {
+for (var k = 0; k < 255; k++) {
-	QRMath.LOG_TABLE[QRMath.EXP_TABLE[i] ] = i;
+	QRMath.LOG_TABLE[QRMath.EXP_TABLE[k] ] = k;
 }

Based on learnings

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between c356dbc and 3eba45e.

⛔ Files ignored due to path filters (1)
  • package-lock.json is excluded by !**/package-lock.json
📒 Files selected for processing (14)
  • package.json (1 hunks)
  • src/api/whatsapp.ts (2 hunks)
  • src/controllers/auth.ts (3 hunks)
  • src/utils/qr-generator.ts (1 hunks)
  • vendor/QRCode/QR8bitByte.js (1 hunks)
  • vendor/QRCode/QRBitBuffer.js (1 hunks)
  • vendor/QRCode/QRErrorCorrectLevel.js (1 hunks)
  • vendor/QRCode/QRMaskPattern.js (1 hunks)
  • vendor/QRCode/QRMath.js (1 hunks)
  • vendor/QRCode/QRMode.js (1 hunks)
  • vendor/QRCode/QRPolynomial.js (1 hunks)
  • vendor/QRCode/QRRSBlock.js (1 hunks)
  • vendor/QRCode/QRUtil.js (1 hunks)
  • vendor/QRCode/index.js (1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
src/controllers/*.ts

📄 CodeRabbit inference engine (CLAUDE.md)

Controllers (initializer.ts for session creation, browser.ts for Puppeteer management, auth.ts for authentication/QR) live under src/controllers

Files:

  • src/controllers/auth.ts
🧠 Learnings (1)
📚 Learning: 2025-09-08T17:49:03.345Z
Learnt from: CR
PR: venomlib/venom#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-08T17:49:03.345Z
Learning: Applies to src/lib/wapi/**/*.js : WAPI layer code is built with webpack and injected into the Puppeteer browser context

Applied to files:

  • src/api/whatsapp.ts
🧬 Code graph analysis (2)
src/api/whatsapp.ts (1)
scripts/create-wrappers.js (1)
  • __dirname (8-8)
src/controllers/auth.ts (1)
src/utils/qr-generator.ts (1)
  • generateASCIIQR (30-118)
🪛 Biome (2.1.2)
vendor/QRCode/QRMath.js

[error] 34-34: Shouldn't redeclare 'i'. Consider to delete it or rename it.

'i' is defined here:

(lint/suspicious/noRedeclare)


[error] 40-40: Shouldn't redeclare 'i'. Consider to delete it or rename it.

'i' is defined here:

(lint/suspicious/noRedeclare)

🔇 Additional comments (9)
package.json (1)

28-30: LGTM!

The build pipeline correctly chains the vendor asset copying step after compiling CJS, ESM, and wrapper code. This ensures the vendor QRCode modules are available at runtime.

vendor/QRCode/QRMode.js (1)

1-6: LGTM!

The QR mode constants are correctly defined using bit flags, following the standard QR code specification.

vendor/QRCode/QRMaskPattern.js (1)

1-10: LGTM!

The mask pattern constants are correctly mapped to their respective integer values (0-7) as per the QR code specification.

vendor/QRCode/QRErrorCorrectLevel.js (1)

1-6: LGTM!

The error correction level constants are correctly mapped according to the QR code specification (L=1, M=0, Q=3, H=2).

src/controllers/auth.ts (2)

16-18: Verify the simplified interface status detection.

The interface status check has been simplified to only check for canvas presence to determine UNPAIRED status. Ensure this logic correctly identifies all UNPAIRED states and doesn't produce false positives.


96-101: Good error handling.

The try/catch block with error logging and fallback to empty string provides robust error handling for QR generation failures.

vendor/QRCode/QRMath.js (1)

1-29: LGTM!

The GF(256) finite field arithmetic implementation is correct and follows the standard algorithm for QR code error correction. The glog and gexp functions properly handle edge cases.

src/api/whatsapp.ts (2)

15-22: Remove the incorrect __dirname warning. The try/catch properly catches the runtime ReferenceError in ESM, so the fallback covers both module systems and no change is needed.

Likely an incorrect or invalid review comment.


73-73: Verify WAPI and middleware asset paths for CJS and ESM outputs
After a local build, confirm that

  • dist/lib/wapi/wapi.js and dist/lib/middleware/middleware.js exist
  • the relative paths (../../lib/wapi/wapi.js at line 73 and ../../lib/middleware/middleware.js at line 81 in src/api/whatsapp.ts) resolve correctly from both dist/cjs/api/whatsapp.js and dist/esm/api/whatsapp.js

Copy link
Copy Markdown

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between a3371fc and b54758c.

📒 Files selected for processing (1)
  • src/controllers/auth.ts (3 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
src/controllers/*.ts

📄 CodeRabbit inference engine (CLAUDE.md)

Controllers (initializer.ts for session creation, browser.ts for Puppeteer management, auth.ts for authentication/QR) live under src/controllers

Files:

  • src/controllers/auth.ts
🧬 Code graph analysis (1)
src/controllers/auth.ts (1)
src/utils/qr-generator.ts (1)
  • generateASCIIQR (30-118)
🔇 Additional comments (2)
src/controllers/auth.ts (2)

3-3: LGTM! Import change aligns with PR objectives.

The switch from external qrcode-terminal to the internal generateASCIIQR implementation supports the PR's goal of making asset loading runtime-agnostic.


16-18: Confirm canvas-only UNPAIRED detection covers all edge cases
I couldn’t find any other DOM wrapper or status checks in auth.ts; manually verify that relying solely on document.querySelector('canvas') correctly captures all UNPAIRED scenarios.

@9cb14c1ec0 9cb14c1ec0 merged commit e22fbf7 into master Oct 8, 2025
3 checks passed
@9cb14c1ec0 9cb14c1ec0 deleted the bugfixtry3 branch October 8, 2025 12:13
@coderabbitai coderabbitai bot mentioned this pull request Mar 9, 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.

1 participant