refactor: improve code maintainability #233
Conversation
…e file presence instead of individual commands.
WalkthroughThe pull request refactors CDN link configuration by extracting hardcoded URL mappings into a centralized constant. Additionally, package.json's activation events are streamlined to trigger only when workspace contains specific files like workflow_settings.yaml or dataform.json, rather than broad command triggers. Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~5 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
src/constants.ts (1)
191-200: Consider adding Subresource Integrity (SRI) hashes for CDN resources.For improved security, consider adding SRI hashes to verify CDN resource integrity and protect against compromised CDN content. This is especially important for production deployments.
Example with SRI attributes:
export const defaultCdnLinks = { highlightJsCssUri: "https://cdnjs.cloudflare.com/ajax/libs/highlight.js/11.9.0/styles/default.min.css", highlightJsCssIntegrity: "sha512-...", // Add SRI hash highlightJsUri: "https://cdnjs.cloudflare.com/ajax/libs/highlight.js/11.9.0/highlight.min.js", highlightJsIntegrity: "sha512-...", // Add SRI hash // ... etc }Generate SRI hashes using:
curl -s <URL> | openssl dgst -sha384 -binary | openssl base64 -A
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
package.json(2 hunks)src/constants.ts(1 hunks)src/extension.ts(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
src/constants.ts (1)
src/utils.ts (2)
getTabulatorThemeUri(363-370)getHighlightJsThemeUri(354-361)
src/extension.ts (1)
src/constants.ts (1)
defaultCdnLinks(191-200)
🔇 Additional comments (2)
package.json (1)
416-416: LGTM!Newline at end of file follows best practices.
src/extension.ts (1)
11-11: LGTM! Excellent refactoring for maintainability.Centralizing CDN links into a dedicated constant in
src/constants.tsimproves maintainability by establishing a single source of truth. The import and initialization are correct.Also applies to: 53-53
| "activationEvents": [ | ||
| "workspaceContains:**/*.sqlx", | ||
| "onCommand:extension.runFilesTagsWtOptionsInRemoteWorkspace", | ||
| "onCommand:extension.runCurrentFile", | ||
| "onCommand:extension.runCurrentFileWtApi", | ||
| "onCommand:extension.runCurrentFileWtDependenciesApi", | ||
| "onCommand:extension.runCurrentFileWtDependentsApi", | ||
| "onCommand:extension.showDependentsInGraph", | ||
| "onCommand:extension.runCurrentFileWtDeps", | ||
| "onCommand:extension.runCurrentFileWtDownstreamDeps", | ||
| "onCommand:extension.showCompiledQueryWtDryRun", | ||
| "onCommand:extension.runTag", | ||
| "onCommand:extension.runTagWtDeps", | ||
| "onCommand:extension.runTagWtDownstreamDeps", | ||
| "onCommand:extension.runTagWtApi", | ||
| "onCommand:extension.runTagWtDependenciesApi", | ||
| "onCommand:extension.runTagWtDependentsApi", | ||
| "onCommand:extension.runFilesTagsWtOptions", | ||
| "onCommand:extension.runFilesTagsWtOptionsApi", | ||
| "onCommand:extension.createNewDataformProject", | ||
| "onCommand:extension.showCompiledQueryInWebView", | ||
| "onCommand:extension.dependencyGraphPanel", | ||
| "onCommand:extension.selectWorkspaceFolder", | ||
| "onCommand:extension.clearExtensionCache" | ||
| "workspaceContains:**/workflow_settings.yaml", | ||
| "workspaceContains:**/dataform.json" | ||
| ], |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Extract and compare command registrations
echo "=== Commands registered in package.json ==="
cat package.json | jq -r '.contributes.commands[].command' 2>/dev/null | sort
echo -e "\n=== Commands registered in extension.ts ==="
find . -name "extension.ts" -o -name "extension.js" | head -5 | while read file; do
if [ -f "$file" ]; then
echo "File: $file"
rg -oP "registerCommand\(['\"]([^'\"]+)" "$file" | sed "s/registerCommand(//" | sed "s/['\"]//g" | sort
fi
done
echo -e "\n=== Activation Events in package.json ==="
cat package.json | jq -r '.activationEvents[]' 2>/dev/nullRepository: ashish10alex/vscode-dataform-tools
Length of output: 2441
🏁 Script executed:
#!/bin/bash
# Search for missing command implementations
echo "=== Searching for showCompiledQueryInWebView in extension.ts ==="
rg -n "showCompiledQueryInWebView" ./src/extension.ts
echo -e "\n=== Searching for fixError in extension.ts ==="
rg -n "fixError" ./src/extension.ts
echo -e "\n=== Searching for runAssertions in extension.ts ==="
rg -n "runAssertions" ./src/extension.ts
echo -e "\n=== Checking package.json for fixError and runAssertions ==="
rg -n "fixError\|runAssertions" ./package.jsonRepository: ashish10alex/vscode-dataform-tools
Length of output: 480
🏁 Script executed:
#!/bin/bash
# Search entire codebase for showCompiledQueryInWebView
echo "=== Searching entire src directory for showCompiledQueryInWebView ==="
rg -rn "showCompiledQueryInWebView" ./src/
echo -e "\n=== Checking if showCompiledQueryInWebView appears in any other files ==="
rg -rn "showCompiledQueryInWebView" ./ --type-add 'web:*.{ts,js,json}' -g '!node_modules' 2>/dev/null | head -20Repository: ashish10alex/vscode-dataform-tools
Length of output: 865
Commands have critical registration/declaration mismatches that must be resolved before the activation event changes are safe.
The verification reveals three issues:
-
Orphaned command:
vscode-dataform-tools.showCompiledQueryInWebViewis declared in package.json but the actual implementation usesvscode-dataform-tools.ninsrc/views/register-preview-compiled-panel.ts. Commands invoked with the wrong ID will fail. -
Missing declarations:
vscode-dataform-tools.fixError(line 177) andvscode-dataform-tools.runAssertions(line 129) are registered insrc/extension.tsbut not declared in package.json. These commands will not appear in the VS Code command palette. -
Inconsistency risk: With the new
workspaceContainsactivation events, the extension will only activate when.sqlx,workflow_settings.yaml, ordataform.jsonfiles are present. If users invoke undeclared or misnamed commands before these files exist, they will fail.
Required fixes:
- Correct package.json command ID from
showCompiledQueryInWebViewtovscode-dataform-tools.n - Add
fixErrorandrunAssertionsto thecontributes.commandssection in package.json - Ensure all command IDs in package.json match their registrations in the implementation files
🤖 Prompt for AI Agents
In package.json around lines 25 to 29, the contributes.commands section has
mismatched and missing command IDs: change the declared command ID currently
listed as vscode-dataform-tools.showCompiledQueryInWebView to match the
implementation ID vscode-dataform-tools.n, and add declarations for
vscode-dataform-tools.fixError and vscode-dataform-tools.runAssertions so they
appear in the command palette; then scan implementation files
(src/views/register-preview-compiled-panel.ts and src/extension.ts) to ensure
every registered command ID exactly matches a package.json contributes.commands
entry and update any remaining mismatches so activation events and command
registrations are consistent.
| export const defaultCdnLinks = { | ||
| highlightJsCssUri: "https://cdnjs.cloudflare.com/ajax/libs/highlight.js/11.9.0/styles/default.min.css", | ||
| highlightJsUri: "https://cdnjs.cloudflare.com/ajax/libs/highlight.js/11.9.0/highlight.min.js", | ||
| highlightJsOneDarkThemeUri: "https://cdnjs.cloudflare.com/ajax/libs/highlight.js/11.9.0/styles/atom-one-dark.min.css", | ||
| highlightJsOneLightThemeUri: "https://cdnjs.cloudflare.com/ajax/libs/highlight.js/11.9.0/styles/atom-one-light.min.css", | ||
| highlightJsLineNoExtUri: "https://cdn.jsdelivr.net/npm/highlightjs-line-numbers.js/dist/highlightjs-line-numbers.min.js", | ||
| tabulatorDarkCssUri: "https://unpkg.com/tabulator-tables@6.2.5/dist/css/tabulator_midnight.min.css", | ||
| tabulatorLightCssUri: "https://unpkg.com/tabulator-tables@6.2.5/dist/css/tabulator_simple.min.css", | ||
| tabulatorUri: "https://unpkg.com/tabulator-tables@6.3.0/dist/js/tabulator.min.js", | ||
| } |
There was a problem hiding this comment.
Fix Tabulator version mismatch between CSS and JavaScript.
The Tabulator CSS files reference version 6.2.5 (lines 197-198) while the JavaScript file references version 6.3.0 (line 199). Mixing versions can cause styling inconsistencies or JavaScript errors due to API/CSS class mismatches between releases.
Apply this diff to align all Tabulator resources to the same version:
- tabulatorDarkCssUri: "https://unpkg.com/tabulator-tables@6.2.5/dist/css/tabulator_midnight.min.css",
- tabulatorLightCssUri: "https://unpkg.com/tabulator-tables@6.2.5/dist/css/tabulator_simple.min.css",
- tabulatorUri: "https://unpkg.com/tabulator-tables@6.3.0/dist/js/tabulator.min.js",
+ tabulatorDarkCssUri: "https://unpkg.com/tabulator-tables@6.3.0/dist/css/tabulator_midnight.min.css",
+ tabulatorLightCssUri: "https://unpkg.com/tabulator-tables@6.3.0/dist/css/tabulator_simple.min.css",
+ tabulatorUri: "https://unpkg.com/tabulator-tables@6.3.0/dist/js/tabulator.min.js",📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export const defaultCdnLinks = { | |
| highlightJsCssUri: "https://cdnjs.cloudflare.com/ajax/libs/highlight.js/11.9.0/styles/default.min.css", | |
| highlightJsUri: "https://cdnjs.cloudflare.com/ajax/libs/highlight.js/11.9.0/highlight.min.js", | |
| highlightJsOneDarkThemeUri: "https://cdnjs.cloudflare.com/ajax/libs/highlight.js/11.9.0/styles/atom-one-dark.min.css", | |
| highlightJsOneLightThemeUri: "https://cdnjs.cloudflare.com/ajax/libs/highlight.js/11.9.0/styles/atom-one-light.min.css", | |
| highlightJsLineNoExtUri: "https://cdn.jsdelivr.net/npm/highlightjs-line-numbers.js/dist/highlightjs-line-numbers.min.js", | |
| tabulatorDarkCssUri: "https://unpkg.com/tabulator-tables@6.2.5/dist/css/tabulator_midnight.min.css", | |
| tabulatorLightCssUri: "https://unpkg.com/tabulator-tables@6.2.5/dist/css/tabulator_simple.min.css", | |
| tabulatorUri: "https://unpkg.com/tabulator-tables@6.3.0/dist/js/tabulator.min.js", | |
| } | |
| export const defaultCdnLinks = { | |
| highlightJsCssUri: "https://cdnjs.cloudflare.com/ajax/libs/highlight.js/11.9.0/styles/default.min.css", | |
| highlightJsUri: "https://cdnjs.cloudflare.com/ajax/libs/highlight.js/11.9.0/highlight.min.js", | |
| highlightJsOneDarkThemeUri: "https://cdnjs.cloudflare.com/ajax/libs/highlight.js/11.9.0/styles/atom-one-dark.min.css", | |
| highlightJsOneLightThemeUri: "https://cdnjs.cloudflare.com/ajax/libs/highlight.js/11.9.0/styles/atom-one-light.min.css", | |
| highlightJsLineNoExtUri: "https://cdn.jsdelivr.net/npm/highlightjs-line-numbers.js/dist/highlightjs-line-numbers.min.js", | |
| tabulatorDarkCssUri: "https://unpkg.com/tabulator-tables@6.3.0/dist/css/tabulator_midnight.min.css", | |
| tabulatorLightCssUri: "https://unpkg.com/tabulator-tables@6.3.0/dist/css/tabulator_simple.min.css", | |
| tabulatorUri: "https://unpkg.com/tabulator-tables@6.3.0/dist/js/tabulator.min.js", | |
| } |
🤖 Prompt for AI Agents
In src/constants.ts around lines 191 to 200 the Tabulator CSS URIs use version
6.2.5 while the Tabulator JS URI uses 6.3.0, causing a version mismatch; update
the tabulatorDarkCssUri and tabulatorLightCssUri to use the same version as the
JS (6.3.0) so all Tabulator resources are aligned, i.e., replace the 6.2.5 in
those two CSS URIs with 6.3.0.
🧩 Analysis chain
🌐 Web query:
highlight.js latest version 2025
💡 Result:
As of Nov 27, 2025 the official highlight.js repository shows the latest release as v11.11.1. [1] Softpedia lists v11.11.2 (Mar 5, 2025) but the upstream GitHub releases page (the authoritative source) lists v11.11.1 as the latest. [1][2]
Sources:
- GitHub releases (highlightjs/highlight.js) — latest tag v11.11.1. [1]
- Softpedia listing showing v11.11.2. [2]
🌐 Web query:
tabulator-tables latest version 2025
💡 Result:
The latest Tabulator (tabulator-tables) release is v6.3.1 (released Jan 19, 2025). [1][2]
Sources:
- GitHub releases page (olifolkerd/tabulator) — 6.3.1. [1]
- Official Tabulator release notes / docs — Version 6.3. [2]
🌐 Web query:
highlight.js 11.9.0 security vulnerabilities
💡 Result:
Short answer: There are no publicly reported, direct security vulnerabilities specific to highlight.js v11.9.0. Current advisories / databases list older issues (prototype pollution and ReDoS) that affect pre-10.x / early 10.x releases and were fixed in later 10.x+ and 11.x releases. [1][2][3][4]
Details and sources:
- Snyk shows no direct vulnerabilities for highlight.js@11.9.0. [1]
- highlight.js security page documents known advisories (Prototype Pollution GHSA-vfrc… / ReDoS GHSA-7wwv…) and states 11.x receives security fixes. [2]
- CVE-2020-26237 (prototype pollution) affected versions before 9.18.2 / 10.1.2 and was fixed. [3]
- GHSA-7wwv-vh3v-89cq (ReDoS) affected versions <10.4.1 and was fixed in later releases. [4]
If you want, I can:
- scan a project (package.json / lockfile) to check whether you actually depend on 11.9.0 or any vulnerable transitive version, or
- fetch the changelog/patches between 11.9.0 and the latest 11.x release.
🌐 Web query:
tabulator-tables security vulnerabilities 2025
💡 Result:
Short answer: There are no public 2025 CVEs for the tabulator-tables npm package itself. Security scanners (Snyk) list no direct vulnerabilities in tabulator-tables (latest v6.3.1). [1]
Notable related issue: a 2024 XSS (CVE-2024-23995) affected Beekeeper Studio via its use of Tabulator's tabulator-popup-container (column-name XSS). That CVE is recorded by NVD and multiple vulnerability databases; it is a product-specific XSS rather than a reported CVE against the tabulator-tables package itself. [2][3]
If you rely on Tabulator in production: keep dependencies updated, subscribe to project advisories / Snyk/GitHub alerts, and treat third-party apps that embed Tabulator (like Beekeeper Studio) as potential sources of downstream vulnerabilities.
Sources:
- Snyk — tabulator-tables (no known security issues). [1]
- NVD / CVE-2024-23995 (Beekeeper Studio XSS via tabulator-popup-container). [2]
- CVE details / vendor advisories covering CVE-2024-23995. [3]
Update CDN library versions to latest releases; align tabulator-tables CSS and JS versions.
The versions specified are outdated:
- highlight.js: Currently pinned to v11.9.0; latest is v11.11.1
- tabulator-tables: CSS pinned to v6.2.5, JS to v6.3.0; latest is v6.3.1
Additionally, tabulator-tables has a version mismatch between CSS (6.2.5) and JS (6.3.0) resources, which may cause compatibility issues. While no direct security vulnerabilities were found in these versions, updating to the latest releases ensures access to recent security patches and bug fixes.
🤖 Prompt for AI Agents
In src/constants.ts around lines 191 to 200, update the pinned CDN versions:
change highlight.js URIs from v11.9.0 to v11.11.1 (both CSS and JS and any
related theme URIs), and align tabulator-tables CSS and JS to the same latest
version v6.3.1 (update both tabulatorDarkCssUri/tabulatorLightCssUri and
tabulatorUri to use 6.3.1) so CSS and JS versions match and use the newest
releases.
Summary by CodeRabbit
Release Notes
✏️ Tip: You can customize this high-level summary in your review settings.