Skip to content

Local NodeWebserverBrowserController updated Node 18-25 #490 - #511

Merged
thias15 merged 6 commits into
ob-f:masterfrom
iwl:brwsctrl
Aug 12, 2026
Merged

thias15 merged 6 commits into
ob-f:masterfrom
iwl:brwsctrl

Conversation

@iwl

@iwl iwl commented Apr 23, 2026

Copy link
Copy Markdown
Contributor

Local NodeWebserverBrowserController updated Node 18-25
OpenBot/controller/node-js npm install npm start
First connect Browser http://Server:8081 , then OpenBot (close app,retry)
Works Linux, Win10, Firewall Open tcp 19400 (8081 extern Browser)
Videostream works, no car yet for control test
Fixes #490

@iwl iwl changed the title Lokal BrowserCtrl Pi 4 Bookworm 32 Node 20 Lokal BrowserCtrl Pi 4 Bookworm 32 Node 20 #490 Apr 23, 2026
@iwl iwl changed the title Lokal BrowserCtrl Pi 4 Bookworm 32 Node 20 #490 Local NodeWebserverBrowserController updated Node 18-25 #490 Apr 24, 2026
@thias15

thias15 commented May 8, 2026

Copy link
Copy Markdown
Collaborator

Overall: Good modernization — Snowpack is unmaintained and this should restore Node 18-25 compatibility. The JSON.parse(data).status change looks like a real bug fix (the previous (data).status was a no-op cast).

  1. End-to-end test on a real bot is needed before merging. The PR description notes "Videostream works, no car yet for control test." The JSON.parse fix sits directly on the message path from the OpenBot app, so control behavior could regress (or finally work) — we shouldn't merge without confirming a live device can drive.
  2. ESLint 9 uses flat config by default. Does npm run lint still pass? v9 dropped support for .eslintrc unless ESLINT_USE_FLAT_CONFIG=false is set, and eslint-config-standard was removed from devDependencies — worth verifying lint still runs cleanly.

@harishthakur52

Copy link
Copy Markdown
Contributor

Overall: Good modernization — Snowpack is unmaintained and this should restore Node 18-25 compatibility. The JSON.parse(data).status change looks like a real bug fix (the previous (data).status was a no-op cast).

  1. End-to-end test on a real bot is needed before merging. The PR description notes "Videostream works, no car yet for control test." The JSON.parse fix sits directly on the message path from the OpenBot app, so control behavior could regress (or finally work) — we shouldn't merge without confirming a live device can drive.
  2. ESLint 9 uses flat config by default. Does npm run lint still pass? v9 dropped support for .eslintrc unless ESLINT_USE_FLAT_CONFIG=false is set, and eslint-config-standard was removed from devDependencies — worth verifying lint still runs cleanly.

Live bot / control

  • Confirmed working end-to-end on a real device
  • Fixed via JSON.parse(data).status on the OpenBot message path

ESLint

  • npm run lint was broken: ESLint 9 needs flat config, and eslint-config-standard had
    been dropped
  • Fixed by reverting to:
    • eslint@^7.32.0
    • eslint-config-standard@^16.0.3
    • eslint-plugin-import@^2.29.0
    • eslint-plugin-node@^11.1.0
    • eslint-plugin-promise@^5.2.0
  • Existing .eslintrc.json now works again — lint passes

CI

  • vite@^5.2.0 requires Node ^18 || >=20
  • .github/workflows/nodejs-server.yml is still pinned to node-version: '16'
  • Needs update to node-version: '20'

@thias15

thias15 commented Aug 1, 2026

Copy link
Copy Markdown
Collaborator

@harishthakur52 is there anything that still needs fixing? Node.js server build and test are not passing...

@thias15

thias15 commented Aug 1, 2026

Copy link
Copy Markdown
Collaborator

Pushed a fix directly to this branch: bumped node-version 16→20 in nodejs-server.yml (Vite 5 needs Node ≥18, that's what was crashing the health check). CI is green now.

Note: npm run lint isn't actually part of CI, so the eslint claim above doesn't block merging, though package.json still shows eslint@^9.0.0, not the reverted 7.x versions mentioned. Was that pushed?

Can you confirm the live-bot test still holds on current head (62d5292)?

@hardikgarg02 hardikgarg02 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Tested for npm run lint and real bot. Approved!

@thias15 thias15 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Both outstanding items from the earlier review are resolved: ESLint reverted to the 7.x line (fixes the flat-config break), and CI is green on the latest commit. Hardik confirmed a live-bot test on this head. Approving.

@thias15
thias15 merged commit b5e555a into ob-f:master Aug 12, 2026
2 checks passed
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.

Uncaught TypeError: Cannot convert undefined or null to object

4 participants