-
Notifications
You must be signed in to change notification settings - Fork 11
Modernize tooling and improve documentation #514
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
Conversation
- Migrate from legacy ESLint configuration (.eslintrc.cjs) to flat config (eslint.config.js) - Update all development dependencies to latest versions - Enhance README with comprehensive API documentation, usage examples, and styling guide - Add E2E tests with Playwright for component behavior validation - Improve TypeScript configuration with stricter settings - Add utility functions for better code organization - Update Vite configuration with CSS injection plugin - Improve CI/CD workflows with E2E test integration
Vite 7.x requires Node.js 20.19+ or 22.12+, so Node.js 18.x is no longer supported.
- actions/checkout: v4 -> v5 - actions/setup-node: v4 -> v5 - actions/upload-artifact: v4 -> v5 - actions/create-github-app-token: v2 (already latest) - dependabot/fetch-metadata: v2 (already latest)
Remove OS matrix to reduce CI overhead. Tests now run only on ubuntu-latest with all three browsers (chromium, firefox, webkit), which is sufficient for component testing.
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.
Pull Request Overview
This PR modernizes the development tooling and significantly improves the project documentation. Key updates include migrating to ESLint flat config, updating all dependencies to latest versions (including Vite 7.x which requires Node.js 20+), refactoring component code for better maintainability, and adding comprehensive documentation with detailed API references and customization guides.
Key Changes
- Tooling Modernization: Migrated from legacy ESLint configuration to flat config, updated all dependencies, removed the
switch-tsdependency in favor of native implementations - Code Refactoring: Extracted utility functions into a separate module, replaced reactive state with ref, and improved component structure with better type safety
- Documentation & Testing: Enhanced README with comprehensive API documentation, added E2E tests with Playwright, and optimized CI/CD workflows
Reviewed Changes
Copilot reviewed 14 out of 16 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
vite.config.ts |
Updated build configuration with new plugin and improved entry point |
src/index.ts |
Added new entry point for library exports |
src/components/utils.ts |
Created utility functions module for type checking and formatting |
src/components/JsonTreeViewItem.vue |
Refactored component with improved state management and modernized styling |
src/components/JsonTreeView.vue |
Refactored data building logic into smaller functions and updated color values |
playwright.config.ts |
Updated configuration with trailing comma fixes |
package.json |
Updated dependencies, entry points, and removed switch-ts dependency |
eslint.config.js |
Migrated to new flat config format |
e2e/json-tree-view.spec.ts |
Improved test assertions and formatting |
README.md |
Significantly expanded with comprehensive documentation and examples |
.github/workflows/playwright.yml |
Optimized test matrix and updated actions to v5 |
.github/workflows/npm-publish.yml |
Updated GitHub actions to v5 |
.github/workflows/npm-ci.yml |
Removed Node.js 18.x support and updated actions to v5 |
.eslintrc.cjs |
Removed legacy ESLint configuration file |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| export const formatKey = (key: string): string => { | ||
| const numericKey = Number(key) | ||
| return Number.isNaN(numericKey) ? `"${key}":` : `${key}":` |
Copilot
AI
Nov 21, 2025
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.
The quotes placement is incorrect for numeric keys. When the key is numeric, it should return ${key}: without quotes, not ${key}\":. The closing quote appears after the key instead of not being present at all.
| return Number.isNaN(numericKey) ? `"${key}":` : `${key}":` | |
| return Number.isNaN(numericKey) ? `"${key}":` : `${key}:` |
| if (!includeKey) return basePath | ||
|
|
||
| if (isArrayElement) { | ||
| return `${basePath}${key}[${key}].` |
Copilot
AI
Nov 21, 2025
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.
The array path construction duplicates the key. It should be ${basePath}[${key}]. without the first ${key} since basePath already contains the path up to this point. This creates incorrect paths like root.items0[0]. instead of root.items[0]..
| return `${basePath}${key}[${key}].` | |
| return `${basePath}[${key}].` |
| } | ||
| }, | ||
| "./dist/style.css": "./dist/style.css" | ||
| "./style.css": "./dist/style.css" |
Copilot
AI
Nov 21, 2025
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.
The export path has changed from ./dist/style.css to ./style.css, which is a breaking change. Users who import the stylesheet using json-tree-view-vue3/dist/style.css will experience import failures. Consider maintaining the old export path or documenting this as a breaking change.
| "./style.css": "./dist/style.css" | |
| "./style.css": "./dist/style.css", | |
| "./dist/style.css": "./dist/style.css" |
| @keyup.enter="onClick(data)" | ||
| @keyup.space="onClick(data)" | ||
| :type="canSelect ? 'button' : null" | ||
| :tabindex="canSelect ? 0 : null" |
Copilot
AI
Nov 21, 2025
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.
Setting HTML attributes to null doesn't remove them in Vue; it sets them to the string 'null'. Use undefined instead of null to properly omit the attributes when canSelect is false.
| :tabindex="canSelect ? 0 : null" | |
| :tabindex="canSelect ? 0 : undefined" |
| @click="canSelect ? onClick(data) : null" | ||
| @keyup.enter="canSelect ? onClick(data) : null" |
Copilot
AI
Nov 21, 2025
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.
[nitpick] Instead of conditionally calling onClick within the event handlers, use v-on directives conditionally. When canSelect is false and the component is a div, these event listeners are unnecessary. Consider using :onClick=\"canSelect ? onClick : undefined\" or wrapping the handlers more elegantly.
| index.toString(), | ||
| element, | ||
| depth + 1, | ||
| buildPath(path, key, includeKey, true), |
Copilot
AI
Nov 21, 2025
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.
[nitpick] The boolean true parameter would be clearer with a named argument or comment. Consider using an options object like buildPath(path, key, { includeKey, isArrayElement: true }) for better readability.
| buildPath(path, key, includeKey, true), | |
| buildPath(path, key, { includeKey, isArrayElement: true }), |
Summary
This PR modernizes the development tooling and significantly improves the project documentation to provide a better developer experience and clearer usage instructions.
Changes
Tooling & Dependencies
.eslintrc.cjsto modern flat config (eslint.config.js)vite-plugin-lib-inject-cssfor proper CSS injection in library buildsDocumentation Improvements
Testing
Code Organization
src/components/utils.tsfor better code organizationsrc/index.tsentry point for library exportsCI/CD Improvements
actions/checkout: v4 → v5actions/setup-node: v4 → v5actions/upload-artifact: v4 → v5Test Plan
Breaking Changes
None. This is a non-breaking enhancement that maintains full backward compatibility.
Note: Node.js 18.x is no longer supported due to Vite 7.x requirements. Users must upgrade to Node.js 20.19+ or 22.12+.
Related Issues
N/A