feat(release): add dist directory check for specific packages to optimize the release process #3265
Conversation
…mize the release process
WalkthroughThis change introduces a new array constant, Changes
Sequence Diagram(s)sequenceDiagram
participant R as releaseAlpha
participant F as fs.existsSync
participant P as findAllpage
R->>R: Iterate over distLists
alt Package in needDistDirPackages?
R->>F: Check "dist" directory
alt "dist" exists
R->>P: Call findAllpage with package path & updateVersion
else "dist" missing
R->>F: Check "dist3" directory
alt "dist3" exists
R->>P: Call findAllpage with package path & updateVersion
else
R->>R: Skip findAllpage call
end
end
else
R->>P: Directly call findAllpage with package path
end
Suggested labels
Suggested reviewers
Poem
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
internals/cli/src/commands/release/releaseAlpha.tsOops! Something went wrong! :( ESLint: 8.57.1 ESLint couldn't find the plugin "eslint-plugin-vue". (The package "eslint-plugin-vue" was not found when loaded as a Node module from the directory "".) It's likely that the plugin isn't installed correctly. Try reinstalling by running the following: The plugin "eslint-plugin-vue" was referenced from the config file in ".eslintrc.js » @antfu/eslint-config » @antfu/eslint-config-vue". If you still can't figure out the problem, please stop by https://eslint.org/chat/help to chat with the team. ✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
|
[e2e-test-warn] The title of the Pull request should look like "fix(vue-renderless): [action-menu, alert] fix xxx bug". Please make sure you've read our contributing guide |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🔭 Outside diff range comments (1)
internals/cli/src/commands/release/releaseAlpha.ts (1)
81-107: 💡 Verification agent🧩 Analysis chain
Consider adding tests for the new functionality.
The PR adds new conditional logic to the release process. It would be beneficial to have tests to verify this behavior.
🌐 Web query:
How to write tests for Node.js file system operations with fs-extra?💡 Result:
To write effective tests for Node.js file system operations using
fs-extra, leverage its promise-based API and implement robust setup/teardown workflows. Here's a structured approach:Core Testing Strategy
- Isolate tests using temporary directories
- Use async/await for clean promise handling
- Verify file operations with explicit assertions
- Clean up resources after each test
Example Test Structure (Mocha/Jest)
const fs = require('fs-extra'); const path = require('path'); const { expect } = require('chai'); describe('File System Operations', () => { let tempDir; beforeEach(async () => { tempDir = path.join(__dirname, 'temp-' + Date.now()); await fs.ensureDir(tempDir); // Create unique directory [4][5] }); afterEach(async () => { await fs.remove(tempDir); // Cleanup after tests [1][4] }); it('should create and verify file', async () => { const testFile = path.join(tempDir, 'test.txt'); await fs.outputFile(testFile, 'sample content'); // Create file [2][4] const exists = await fs.pathExists(testFile); const content = await fs.readFile(testFile, 'utf8'); expect(exists).to.be.true; expect(content).to.equal('sample content'); }); it('should handle recursive directory creation', async () => { const nestedPath = path.join(tempDir, 'a/b/c'); await fs.ensureDir(nestedPath); // Create nested directories [1][4] const dirExists = await fs.pathExists(nestedPath); expect(dirExists).to.be.true; }); });Key Best Practices
Test Isolation
- Create unique temporary directories for each test run (
Date.now()in path)- Use
beforeEach/afterEachhooks for setup/teardown [7]Error Handling
it('should throw on invalid operations', async () => { await expect(fs.readFile('nonexistent.txt')) .to.be.rejectedWith('ENOENT'); });File Contents Verification
- Use checksum validation for binary files
- Compare JSON structures with deep equality checks
Cross-Platform Considerations
- Use
path.join()for file paths- Test symlink behavior with platform-specific conditions
- Set
CROSS_DEVICE_PATHenv var for move operations [1][5]Advanced Testing Patterns
Benchmarking Operations
it('should meet performance thresholds', async function() { this.timeout(5000); const start = Date.now(); await fs.copy(largeDir, tempCopyDir); const duration = Date.now() - start; expect(duration).to.be.below(1000); });Parallel Execution Safety
- Use separate directory hierarchies for parallel test runs
- Avoid shared state between test files
CI Pipeline Setup
- Add filesystem tests to your continuous integration workflow
- Include cross-platform testing in matrix builds (Linux, Windows, macOS)
For complex scenarios, consider combining
fs-extrawith temporary directory libraries liketmp-promiseand assertion libraries likesinonfor monitoring file system interactions.Citations:
- 1: https://www.npmjs.com/package/fs-extra
- 2: https://www.lambdatest.com/automation-testing-advisor/javascript/cypress-fsExtra.outputFile
- 3: https://javascript.plainenglish.io/5-node-js-file-system-tricks-every-developer-should-know-aa1d150c8966
- 4: https://userjot.com/blog/nodejs-fs-extra-guide/
- 5: https://classic.yarnpkg.com/en/package/fs-extra
- 6: https://unpkg.com/browse/fs-extra@0.26.4/README.md
- 7: https://blog.appsignal.com/2024/10/16/best-testing-practices-in-nodejs.html
- 8: https://www.digitalocean.com/community/tutorials/how-to-work-with-files-using-the-fs-module-in-node-js
- 9: https://nodejs.org/api/fs.html
- 10: https://www.clouddefense.ai/code/javascript/example/fs-extra
Action: Enhance Testing Coverage for Conditional Release Logic
The new changes in
internals/cli/src/commands/release/releaseAlpha.ts(lines 81–107) introduce additional file system checks to determine which packages trigger thefindAllpagefunction. To ensure the intended behavior:
- Simulate Various Scenarios: Create tests that mimic conditions where the "dist" or "dist3" directories exist (or not) to validate that packages within
needDistDirPackagescorrectly evaluate the directory existence before callingfindAllpage.- Cover All Branches: For packages not in
needDistDirPackages, confirm thatfindAllpageis always invoked regardless of the file system state.- Leverage fs-extra Best Practices: Use temporary directories (with libraries like
fs-extra) along with async/await for clean setup and teardown. This approach will isolate file system operations and help assert the correctness of file existence checks and subsequent function calls.Adopting a testing strategy, such as the Mocha/Jest examples provided in the recent best practices, will ensure robust verification of these conditional code paths.
🧹 Nitpick comments (3)
internals/cli/src/commands/release/releaseAlpha.ts (3)
94-104: The conditional logic provides an optimization to the release process.The added check ensures that packages like 'utils', 'vue-hooks', and 'vue-runtime' are only processed if they have the necessary build output (a 'dist' or 'dist3' directory). This prevents unnecessary work when these packages haven't been built yet.
Consider adding a brief comment explaining why these specific packages require the dist directory check for better maintainability:
80 const needDistDirPackages = ['utils', 'vue-hooks', 'vue-runtime'] + + // These packages need to have their dist/dist3 directories checked before processing + // because they require a build step to generate output files
96-98: Consider adding error handling for file system operations.While
existsSyncdoesn't throw exceptions, it's good practice to add error handling for file system operations.- const hasDistDir = fs.existsSync(pathFromPackages(`${item}/dist`)) - const hasDist3Dir = fs.existsSync(pathFromPackages(`${item}/dist3`)) + let hasDistDir = false; + let hasDist3Dir = false; + try { + hasDistDir = fs.existsSync(pathFromPackages(`${item}/dist`)) + hasDist3Dir = fs.existsSync(pathFromPackages(`${item}/dist3`)) + } catch (error) { + console.error(`Error checking dist directories for ${item}:`, error) + }
94-104: Consider adding a log message for skipped packages.For debugging purposes, it would be helpful to log when a package is skipped because it doesn't have the required directories.
if (hasDistDir || hasDist3Dir) { findAllpage(pathFromPackages(item), updateVersion) + } else { + console.log(`Skipping ${item} as it doesn't have dist or dist3 directory`) }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
internals/cli/src/commands/release/releaseAlpha.ts(2 hunks)
🧰 Additional context used
🪛 Biome (1.9.4)
internals/cli/src/commands/release/releaseAlpha.ts
[error] 80-81: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
🔇 Additional comments (2)
internals/cli/src/commands/release/releaseAlpha.ts (2)
80-80: New constant created for packages requiring dist directory check.This constant defines packages that need special handling during the release process. It's a good practice to extract this list as a constant for maintainability.
🧰 Tools
🪛 Biome (1.9.4)
[error] 80-81: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
75-75: Static analysis hint related to delete operator is a false positive.The static analysis tool flagged the use of
deleteoperator, but this is a standard way to remove properties from objects in JavaScript, and the performance impact is negligible in this context.
…mize the release process (#3265)
* feat(release): add dist directory check for specific packages to optimize the release process (#3265) * fix(base-select): [select,base-select] fix the highlight style of the selected item icon (#3268) * fix(base-select): [select,base-select] fix the highlight style of the selected item icon * feat(base-select): [base-select] modify version number * chore: release v3.22.1 (#3275) * chore: added the cdn address of Huicharts component runtime, and updated the version number of docs to 3.22.3 --------- Co-authored-by: ajaxzheng <894103554@qq.com> Co-authored-by: MomoPoppy <125256456+MomoPoppy@users.noreply.github.com>
增加对特定包的 dist 目录检查，优化发布流程
PR
PR Checklist
Please check if your PR fulfills the following requirements:
PR Type
What kind of change does this PR introduce?
What is the current behavior?
Issue Number: N/A
What is the new behavior?
Does this PR introduce a breaking change?
Other information
Summary by CodeRabbit