Conversation
…trol feat: add file size validation in FileSystemService test: add contract validation tests for Barrel types and enums chore: update ESLint rules for TypeScript annotations
…el file generation
…te logic and fix critical bug
…r concurrency control - Added unit tests for array utilities, including `isEmptyArray`. - Implemented comprehensive tests for custom assertion utilities. - Created tests for error handling utilities, including `getErrorMessage` and `formatErrorForLog`. - Developed tests for ESLint plugin helpers and string utilities. - Introduced a Semaphore class to manage concurrency with a limit on concurrent operations. - Added tests for the Semaphore class to ensure correct behavior in queuing and releasing permits. - Updated string utility functions with improved sorting logic and added tests for various scenarios. - Added type definitions for Logger and OutputChannel interfaces.
… and update tests for consistency
…access types; add hardening plan for test coverage
…actor extension tests for improved clarity
… inclusion, and update README logo
- Bump version in package.json from 1.1.0 to 1.1.1. - Enhance BarrelContentBuilder to support TypeScript 4.5+ mixed export syntax, combining value and type exports in a single statement. - Refactor export patterns to handle comments and whitespace variations in export statements. - Update tests to validate new export handling, including mixed exports and comments. - Remove deprecated test files and clean up unused imports in various modules. - Improve logging module exports for better clarity and organization.
…g logging in content sanitizer
…ernals configuration for ts-morph
…relContentSanitizer
Contributor
There was a problem hiding this comment.
Pull request overview
This pull request delivers version 1.1.1 of the "barrel-roll" extension, focusing on robust export parsing improvements, TypeScript 4.5+ compatibility, and test infrastructure modernization. The changes enhance the extension's ability to handle edge cases in export statements (particularly comments containing special characters) and consolidate to using TypeScript's modern mixed export syntax.
Changes:
- Enhanced export pattern regex to use non-greedy matching, fixing duplication bugs when comments contain closing braces, and adding support for type-only exports, varied whitespace, and exports without spaces
- Adopted TypeScript 4.5+ mixed export syntax (
export { value, type Type }) throughout barrel generation and consolidated barrel export files - Removed Jest compatibility shim and Jest-specific dependencies, fully standardizing on Node.js test runner with 312 comprehensive tests covering export edge cases
Reviewed changes
Copilot reviewed 22 out of 25 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/core/barrel/export-patterns.ts | Updated regex patterns to use non-greedy matching and handle comments, type-only exports, and varied whitespace |
| src/test/unit/core/barrel/export-patterns.test.ts | Added 143 new test cases covering export pattern edge cases including comments, whitespace, and invalid statements |
| src/test/unit/core/barrel/content-sanitizer.test.ts | Added 188 new test cases for content sanitizer with comment handling scenarios |
| src/core/barrel/barrel-content.builder.ts | Implemented TypeScript 4.5+ mixed export syntax for combining value and type exports |
| src/test/unit/core/barrel/barrel-content.builder.test.ts | Added test for mixed export syntax generation |
| src/test/unit/core/barrel/barrel-content.builder.smoke.test.ts | Updated test expectations for mixed export syntax |
| src/types/index.ts | Consolidated to mixed export syntax pattern |
| src/logging/index.ts | Consolidated to mixed export syntax pattern |
| src/core/barrel/index.ts | Added missing type exports to barrel |
| src/extension.ts | Added error rethrow comment (contains issue - see comment) |
| src/test/testHarness.ts | Removed Jest compatibility shim (122 lines deleted) |
| src/test/index.ts | Removed test barrel export file (31 lines deleted) |
| src/test/unit/core/io/file-system.service.test.ts | Removed Jest imports, cleaned up afterEach |
| webpack.config.cjs | Changed externals to array format and added conditional devtool |
| package.json | Removed eslint-plugin-jest and expect dependencies, bumped version to 1.1.1 |
| package-lock.json | Updated dependency versions and removed Jest-related packages |
| .gitignore | Added .depcheck.json |
| CHANGELOG.md | Documented all changes for v1.1.1 |
| badges/coverage.svg | Updated coverage badge to 98.87% |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+53
to
+54
| // Rethrow so the processing loop can observe/log the failure. | ||
| throw error; |
There was a problem hiding this comment.
The throw error after reject(error) will cause an unhandled promise rejection. When an error is thrown from within the Promise executor callback (lines 47-56), it creates an unhandled rejection because the promise has already been rejected. The processQueue() method at line 75 will catch this error and log it, but the original promise returned to the caller will also reject properly via reject(error) at line 52.
Remove the throw error statement. The error is already properly propagated to the caller via reject(error), and the processQueue() method doesn't need to observe it since it's already being handled by the caller.
Suggested change
| // Rethrow so the processing loop can observe/log the failure. | |
| throw error; |
…ing and performance enhancements
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Pull Request
Description
This pull request delivers a significant update (v1.1.1) to the "barrel-roll" extension, focusing on improving export parsing robustness, TypeScript compatibility, and overall codebase clarity. The changes address edge cases with export statements (especially involving comments and type-only exports), modernize export syntax for TypeScript 4.5+, and streamline both code and test infrastructure. Additionally, comprehensive new tests ensure correctness for a wide range of scenarios.
Export parsing and TypeScript compatibility improvements:
Fixed export duplication issues when comments containing closing braces (
}) were present in imports, and enhanced regex patterns inexport-patterns.tsto use non-greedy matching, supporting comments and various export syntaxes. Added support for trailing line/block comments, type-only exports (export type { ... }), comments between export parts, and exports without spaces (e.g.,export{). [1] [2] [3] [4] [5]Updated
BarrelContentBuilderto emit TypeScript 4.5+ mixed export syntax when both value and type exports exist in a module (e.g.,export { Foo, type Bar } from './mod';). Simplified module-path generation logic for better cross-platform compatibility. [1] [2] [3]Test and infrastructure updates:
src/test/testHarness.ts) and related test barrel export file (src/test/index.ts), as well as Jest-specific dev dependencies frompackage.json. [1] [2] [3] [4]Barrel and export organization:
src/core/barrel/index.ts,src/types/index.ts, andsrc/logging/index.tsfor improved clarity and type/value export consistency. [1] [2] [3]src/types/index.tsand related files for better organization.Miscellaneous improvements:
.gitignoreto exclude.depcheck.jsonandwebpack-stats.json, and improved Webpack externals configuration to avoid bundling large dependencies.Removed/cleaned up:
These changes collectively make the extension more robust, easier to maintain, and better aligned with modern TypeScript practices.
Type of Change
Checklist
npm run lintandnpm run format:checkRelated Issues