chore: add development scripts and tooling #139
Conversation
New developer tooling to improve workflows: **New Scripts:** - emulator-wrapper.js - Wrapper for Firebase emulator management - update-maps-from-tarkovdev.js - Automated map metadata sync from Tarkov.dev - SCRIPTS.md documentation updates These scripts improve developer experience and automation without affecting production application code. Part of PR #111 split strategy - subset #4 (scripts/tooling). Related: #111, #133
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. 📝 WalkthroughWalkthroughDocumentation update simplifies scripts/SCRIPTS.md lint section description. Two new utility scripts added: emulator-wrapper.js wraps Firebase emulators with signal handling and debug log cleanup; update-maps-from-tarkovdev.js fetches maps from tarkov.dev with retry logic, converts schema, and preserves existing rotation/bounds data. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant EmulatorWrapper as emulator-wrapper.js
participant Firebase as Firebase Emulator
participant OS as OS Signals
User->>EmulatorWrapper: Run script
EmulatorWrapper->>EmulatorWrapper: Setup signal handlers (SIGINT, SIGTERM)
EmulatorWrapper->>Firebase: Spawn emulators:start
Firebase->>Firebase: Running...
OS->>EmulatorWrapper: Signal received
EmulatorWrapper->>EmulatorWrapper: Cleanup: Delete debug logs
EmulatorWrapper->>Firebase: Exit
EmulatorWrapper->>User: Exit with status
sequenceDiagram
participant Script as update-maps-from-tarkovdev.js
participant FileSystem as File System
participant TarkovDev as tarkov.dev API
Script->>FileSystem: Read existing maps.json
Script->>TarkovDev: Fetch maps (with retry & backoff)
alt Fetch Success
TarkovDev-->>Script: Raw maps data
Script->>Script: Convert to internal schema
Script->>Script: Preserve rotation & bounds
Script->>Script: Handle Factory/Labs special cases
Script->>FileSystem: Write converted maps.json
Script->>Script: Log results
else Fetch Failure
TarkovDev-->>Script: Error
Script->>Script: Retry with backoff (up to limit)
Script->>Script: Log error & exit
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Suggested labels
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
Comment |
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
There was a problem hiding this comment.
Pull Request Overview
This PR introduces two new utility scripts for development workflow automation and updates the scripts documentation to reflect recent changes.
- New script to sync/update map data from tarkov.dev API as fallback data
- New wrapper script to automatically clean up Firebase emulator debug logs
- Updated documentation to accurately describe the lint script behavior
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 6 comments.
| File | Description |
|---|---|
| scripts/update-maps-from-tarkovdev.js | Implements a robust map synchronization script that fetches map data from tarkov.dev with retry logic and transforms it to the application's format while preserving critical coordinate system configurations |
| scripts/emulator-wrapper.js | Provides automatic cleanup of Firebase debug logs when emulators shut down, with proper signal handling for graceful termination |
| scripts/SCRIPTS.md | Updates documentation to accurately reflect that the lint script only runs ESLint, removing outdated references to additional checks |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| * Run: npm run maps:sync | ||
| * Or: node scripts/update-maps-from-tarkovdev.js |
There was a problem hiding this comment.
The documentation references 'npm run maps:sync' but this script is not defined in package.json. Either add the script to package.json or update the documentation to only reference 'node scripts/update-maps-from-tarkovdev.js'.
| * Run: npm run maps:sync | |
| * Or: node scripts/update-maps-from-tarkovdev.js | |
| * Run: node scripts/update-maps-from-tarkovdev.js |
| /** | ||
| * Fetch tarkov.dev maps.json with robust retry logic | ||
| */ |
There was a problem hiding this comment.
The JSDoc comment on lines 76-78 describes 'Fetch tarkov.dev maps.json with robust retry logic' but is placed above the NAME_MAPPING constant, not a fetch function. This comment should either be moved to line 96 above fetchTarkovDevMaps() or updated to describe NAME_MAPPING.
| /** | |
| * Fetch tarkov.dev maps.json with robust retry logic | |
| */ |
| /** | ||
| * Fetch tarkov.dev maps.json with robust retry logic | ||
| */ | ||
| async function fetchTarkovDevMaps() { |
There was a problem hiding this comment.
Duplicate JSDoc comment. The same comment 'Fetch tarkov.dev maps.json with robust retry logic' appears on both line 76-78 and line 93-95. Remove the one at line 76-78.
| await cleanup(); | ||
| process.exit(0); | ||
| }); | ||
|
|
||
| process.on('SIGTERM', async () => { | ||
| console.log('\n📴 Received SIGTERM, shutting down...'); | ||
| await cleanup(); |
There was a problem hiding this comment.
The signal handlers use async functions but process.exit(0) is called immediately after await cleanup(). If the emulatorProcess is still running when a signal is received, it won't be terminated. Consider calling emulatorProcess.kill() before process.exit() to ensure proper cleanup of the child process.
| await cleanup(); | |
| process.exit(0); | |
| }); | |
| process.on('SIGTERM', async () => { | |
| console.log('\n📴 Received SIGTERM, shutting down...'); | |
| await cleanup(); | |
| await cleanup(); | |
| if (emulatorProcess && emulatorProcess.kill) { | |
| emulatorProcess.kill(); | |
| } | |
| process.exit(0); | |
| }); | |
| process.on('SIGTERM', async () => { | |
| console.log('\n📴 Received SIGTERM, shutting down...'); | |
| await cleanup(); | |
| if (emulatorProcess && emulatorProcess.kill) { | |
| emulatorProcess.kill(); | |
| } |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
TarkovTracker/scripts/SCRIPTS.md
Lines 52 to 54 in 62e7d03
Emulator wrapper never wired into npm scripts
The documentation now claims “All emulator scripts now use an intelligent wrapper” but none of the npm scripts were updated to execute scripts/emulator-wrapper.js—package.json still calls firebase emulators:start directly. As a result the new cleanup logic never runs and the debug logs remain, while developers are led to believe the wrapper is active. Either update the npm scripts to invoke the wrapper or adjust the docs to avoid the false promise.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| import { spawn } from 'child_process'; | ||
| import { rm } from 'fs/promises'; | ||
| import { glob } from 'glob'; | ||
|
|
There was a problem hiding this comment.
The new emulator wrapper imports glob but the repository does not list glob anywhere in its dependencies. On a clean install (npm install --omit=dev or after dependency pruning) running node scripts/emulator-wrapper.js will fail immediately with Cannot find package 'glob'… instead of starting the emulators, so the wrapper cannot be used reliably. Please add glob to devDependencies (or swap to an existing utility) before relying on this script.
Useful? React with 👍 / 👎.
coderabbitai
bot
added
type:documentation
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
**P1 Critical - Add Missing Dependency:** - Add 'glob' to devDependencies (required by emulator-wrapper.js) - Add 'maps:sync' npm script for convenience **Emulator Wrapper Fixes:** - Fix signal handlers to kill child process before exit - Fix exit code propagation for signal terminations - Prevent masking failures when process killed by signal **Documentation Fixes:** - Remove duplicate JSDoc comment for fetchTarkovDevMaps() - Move NAME_MAPPING JSDoc to correct location - Remove orphaned comment fragment Addresses all feedback from Copilot AI, CodeRabbit AI, and chatgpt-codex-connector.
There was a problem hiding this comment.
Pull Request Overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if (typeof code === 'number') { | ||
| process.exit(code); | ||
| } | ||
| // Signal or unknown termination – propagate as failure |
There was a problem hiding this comment.
[nitpick] Use an em dash (—) instead of en dash (–) for better punctuation consistency, or use two hyphens (--) which is more common in code comments.
| // Signal or unknown termination – propagate as failure | |
| // Signal or unknown termination -- propagate as failure |
| if (ourKey === 'factory') { | ||
| floors.push('Basement', 'Ground_Floor', 'Second_Floor', 'Third_Floor'); | ||
| } else { |
There was a problem hiding this comment.
The Factory floor names are hardcoded here. If the floor structure changes in the source data from tarkov.dev, this hardcoded list could become stale. Consider adding a comment explaining why Factory requires special handling and under what conditions this hardcoded list should be updated.
| if (ourKey === 'factory') { | |
| floors.push('Basement', 'Ground_Floor', 'Second_Floor', 'Third_Floor'); | |
| } else { | |
| // NOTE: The Factory floor names are hardcoded here because the source data from tarkov.dev | |
| // may be inconsistent or not provide the desired order. If the floor structure, names, or order | |
| // for Factory change in tarkov.dev, this list MUST be updated to match. See CodeQL rule for details. | |
| if (ourKey === 'factory') { | |
| floors.push('Basement', 'Ground_Floor', 'Second_Floor', 'Third_Floor'); |
| > **Purpose**: Documents all npm scripts and development workflows for the | ||
| > TarkovTracker monorepo | ||
There was a problem hiding this comment.
[nitpick] The line break in the middle of the Purpose description appears unnecessary and reduces readability. Consider keeping it on a single line like it was originally, unless there's a specific markdown formatting requirement.
| > **Purpose**: Documents all npm scripts and development workflows for the | |
| > TarkovTracker monorepo | |
| > **Purpose**: Documents all npm scripts and development workflows for the TarkovTracker monorepo |
| Scripts are | ||
| organized to avoid confusion and provide clear entry points from the root. |
There was a problem hiding this comment.
[nitpick] The line break between 'Scripts are' and 'organized' appears arbitrary and reduces readability. Consider keeping this sentence on a single line like it was originally.
| Scripts are | |
| organized to avoid confusion and provide clear entry points from the root. | |
| Scripts are organized to avoid confusion and provide clear entry points from the root. |
DysektAI
removed
type:documentation
Summary
New developer tooling to improve local development workflows with zero production impact.
Part of splitting PR #111 into focused, reviewable pieces.
Changes
New Scripts:
scripts/emulator-wrapper.js- Wrapper for Firebase emulator management with better lifecycle handlingscripts/update-maps-from-tarkovdev.js- Automated map metadata sync from Tarkov.dev APIscripts/SCRIPTS.md- Documentation updates for script usageImpact
Benefits
Testing
These scripts are for local development only and don't affect production:
node scripts/emulator-wrapper.jsnode scripts/update-maps-from-tarkovdev.js