feat: add command to install optional features & builder option #40
Conversation
… builder option to run builder after the build, like for typedoc
WalkthroughThe pull request introduces significant updates to the Changes
Possibly related PRs
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 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 (
|
|
Thank you for following the naming conventions! 🙏 |
prisis
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Outside diff range and nitpick comments (14)
packages/packem/src/rollup/utils/get-entry-file-names.ts (1)
6-9: Approved: Good improvements in variable naming and input handling.The changes here improve code quality:
- Fixing the typo in
pathSeparatorenhances readability.- The new logic correctly handles both array and non-array inputs for
chunkInfo.names.Minor suggestion: Consider updating the comment to reflect the current implementation more accurately.
Consider updating the comment to:
// Handle both array and single-name scenarios for chunkInfo.namespackages/packem/src/utils/find-packem-file.ts (2)
5-16: LGTM: Config file search logic is efficient.The logic for searching configuration files is well-implemented. The use of
for-awaitwithisAccessibleis an efficient way to check file existence asynchronously.Consider extracting the array of config file names to a constant outside the function. This would improve readability and allow easier modification of supported config file types in the future.
const PACKEM_CONFIG_FILES = ["packem.config.js", "packem.config.mjs", "packem.config.cjs", "packem.config.ts", "packem.config.cts", "packem.config.mts"]; // Then in the function: for await (const file of PACKEM_CONFIG_FILES) { // ... existing code }
18-20: LGTM: File extension validation is thorough.The validation logic for file extensions is correct and covers all supported types. The error message is clear and informative.
Consider extracting the regex pattern to a named constant for improved readability:
const VALID_CONFIG_EXTENSION_REGEX = /\.(?:js|mjs|cjs|ts|cts|mts)$/; if (!VALID_CONFIG_EXTENSION_REGEX.test(packemConfigFilePath)) { // ... existing code }This change would make the code more self-documenting and easier to maintain.
packages/packem/src/cli.ts (1)
34-34: LGTM: New command registered correctly.The
createAddCommand(cli)call is correctly placed and follows the existing pattern for command registration.Consider grouping all command registrations together for better readability:
const commands = [createInitCommand, createBuildCommand, createAddCommand]; commands.forEach((command) => command(cli));This approach would make it easier to add or remove commands in the future.
packages/packem/src/types.ts (3)
183-183: LGTM: New properties added to BuildOptions interface.The new
builderandisolatedDeclarationTransformerproperties are well-defined and align with the PR objectives. TheisolatedDeclarationTransformeris correctly marked as experimental.Consider adding JSDoc comments to these new properties to provide more context on their usage and purpose, especially for the experimental
isolatedDeclarationTransformer.Also applies to: 206-207
241-242: LGTM: New builder hooks added and typedoc hooks deprecated.The addition of
builder:beforeandbuilder:donehooks, along with the deprecation oftypedochooks, aligns well with the newbuilderproperty in BuildOptions. This change provides a more generic approach to build steps.Consider adding a brief comment above the new
builderhooks to explain their purpose and how they relate to thebuilderproperty in BuildOptions.Also applies to: 254-257
Line range hint
1-338: Overall assessment: Well-structured changes that align with PR objectives.The changes to this file are focused and consistent with the PR objectives. The new
builderproperty and related hooks provide a flexible way to add custom build steps, which should facilitate the installation of optional features as mentioned in the PR summary.As the build system evolves, consider creating a separate file for build-related types if this file grows too large. This would improve maintainability and make it easier for developers to find and understand the build-related types.
packages/packem/package.json (3)
251-251: Consider adding a version range for the new dependency.The addition of
tinyglobbyis noted. However, it's recommended to specify a version range to ensure consistent behavior across different environments and to prevent potential breaking changes from future updates.Consider updating the line to include a version range, for example:
- "tinyglobby": "0.2.9" + "tinyglobby": "^0.2.9"
260-260: LGTM: New TypeDoc-related devDependencies added.The addition of TypeDoc-related devDependencies aligns with the new functionality mentioned in the AI summary. Using specific versions ensures reproducibility, which is good.
Consider using caret ranges (^) for these dependencies to allow for easier non-breaking updates:
- "@ckeditor/typedoc-plugins": "44.0.0", + "@ckeditor/typedoc-plugins": "^44.0.0", - "typedoc": "0.26.10", + "typedoc": "^0.26.10", - "typedoc-plugin-markdown": "4.2.9", + "typedoc-plugin-markdown": "^4.2.9", - "typedoc-plugin-rename-defaults": "0.7.1", + "typedoc-plugin-rename-defaults": "^0.7.1",Also applies to: 299-301
306-306: LGTM: New TypeDoc-related peerDependencies added and marked as optional.The addition of TypeDoc-related peerDependencies and marking them as optional in
peerDependenciesMetais a good practice, allowing users to install only what they need.Consider using version ranges for peerDependencies to allow for more flexibility:
- "@ckeditor/typedoc-plugins": "44.0.0", + "@ckeditor/typedoc-plugins": "^44.0.0", - "typedoc": "0.26.10", + "typedoc": "^0.26.10", - "typedoc-plugin-markdown": "4.2.9", + "typedoc-plugin-markdown": "^4.2.9", - "typedoc-plugin-rename-defaults": "0.7.1", + "typedoc-plugin-rename-defaults": "^0.7.1",Also applies to: 311-313, 317-319, 332-340
packages/packem/README.md (1)
Line range hint
59-180: Consider enhancing the TypeScript exampleThe project preparation section is informative and well-structured. To further improve it, consider adding a brief explanation of the
typeVersionsfield in the TypeScript example. This would help users understand its purpose and how it relates to Node.js version compatibility.packages/packem/src/commands/add.ts (1)
38-38: Use consistent method to obtain the current working directoryIn line 22, you use
cwd()to get the current working directory, but in line 38, you useprocess.cwd(). For consistency and clarity, consider usingcwd()in both places.Apply this diff to make the change:
- await installPackage(typedocPackages, { cwd: process.cwd(), dev: true, silent: true }); + await installPackage(typedocPackages, { cwd: cwd(), dev: true, silent: true });packages/packem/src/create-bundler.ts (2)
622-622: Clarify variable naming for input configurationIn line 622, the destructuring of
inputConfigresults inotherInputConfig, which is then passed asinputConfigto thecreateContextfunction in line 710. This can be confusing because the variable changes names, potentially impacting readability. Consider renamingotherInputConfigto something likerestInputConfigfor clarity.Apply this diff to improve variable naming:
- const { configPath, debug, tsconfigPath, ...otherInputConfig } = inputConfig; + const { configPath, debug, tsconfigPath, ...restInputConfig } = inputConfig; ... - otherInputConfig, + restInputConfig,Also applies to: 710-710
Line range hint
622-780: Refactor 'createBundler' function for better maintainabilityThe
createBundlerfunction spans over 150 lines and handles multiple responsibilities, including configuration loading, context creation, building processes, and executing builders. This makes it complex and harder to read or maintain. Consider refactoring by extracting distinct functionalities into smaller, well-named helper functions. This will enhance code readability, improve modularity, and make testing and future modifications easier.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (12)
- packages/packem/README.md (1 hunks)
- packages/packem/tests/intigration/typedoc.test.ts (1 hunks)
- packages/packem/package.json (6 hunks)
- packages/packem/packem.config.ts (1 hunks)
- packages/packem/src/builder/typedoc/generate-reference-documentation.ts (1 hunks)
- packages/packem/src/builder/typedoc/index.ts (1 hunks)
- packages/packem/src/cli.ts (2 hunks)
- packages/packem/src/commands/add.ts (1 hunks)
- packages/packem/src/create-bundler.ts (5 hunks)
- packages/packem/src/rollup/utils/get-entry-file-names.ts (1 hunks)
- packages/packem/src/types.ts (4 hunks)
- packages/packem/src/utils/find-packem-file.ts (1 hunks)
✅ Files skipped from review due to trivial changes (2)
- packages/packem/tests/intigration/typedoc.test.ts
- packages/packem/packem.config.ts
🧰 Additional context used
🔇 Additional comments (18)
packages/packem/src/utils/find-packem-file.ts (3)
1-4: LGTM: Imports and function signature are well-defined.The imports and function signature are appropriate for the task. The use of async is suitable for file system operations.
22-25: LGTM: Function return is correct.The function correctly returns the path of the found configuration file or the provided
configPath.
1-25: Overall, excellent implementation with minor suggestions for improvement.The
findPackemFilefunction is well-implemented, efficiently handling the search for Packem configuration files. It covers various scenarios and includes proper error handling. The suggestions provided in previous comments (extracting constants for config file names and the regex pattern) would further enhance code readability and maintainability.To ensure the function works as expected across different environments, consider running the following verification script:
This script will help confirm that the function is being used correctly throughout the project and that all supported config file extensions are consistently referenced.
✅ Verification successful
Verification Successful:
findPackemFileFunctionThe
findPackemFilefunction is correctly imported and utilized across the codebase. All supported configuration file extensions are consistently referenced without any discrepancies.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the findPackemFile function's behavior # Test: Check if the function is imported and used correctly in other files rg -A 5 "findPackemFile" # Test: Verify the supported config file extensions rg "packem\.config\.[jt]" -g "!*.md"Length of output: 3829
packages/packem/src/cli.ts (2)
6-6: LGTM: New command import added correctly.The import statement for
createAddCommandis correctly placed and follows the existing pattern in the file. It maintains consistency with other command imports.
6-6: Verify documentation and test coverage for the new 'add' command.The introduction of the 'add' command enhances the CLI's functionality as per the PR objectives. While the implementation looks correct, ensure that:
- The README or other relevant documentation is updated to include information about this new command.
- Appropriate tests are added to cover the new functionality.
To check if the documentation has been updated, run:
To verify test coverage, run:
Also applies to: 34-34
packages/packem/src/builder/typedoc/generate-reference-documentation.ts (2)
Line range hint
1-138: File structure and consistency maintained.The changes to the import paths have been correctly applied, and the rest of the file remains consistent with these changes. The
generateReferenceDocumentationfunction uses the imported types and utility function correctly.
8-9: LGTM! Verify new import paths.The import path changes look good and reflect a project directory restructure. The functionality should remain unchanged.
Please run the following script to ensure the new import paths are correct:
✅ Verification successful
Import paths verified.
The new import paths correctly reference existing files in the codebase.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the existence of the imported files at their new locations # Test 1: Check if the types file exists fd --type f --full-path '../../types.(ts|js)' --exec echo "Types file found: {}" # Test 2: Check if the replace-content-within-marker utility exists fd --type f --full-path '../../utils/replace-content-within-marker.(ts|js)' --exec echo "Utility file found: {}"Length of output: 197
Script:
#!/bin/bash # Description: Verify the existence of the imported files at their new locations # Test 1: Check if any types file exists within the 'types' directory fd "types.ts" "types.js" ./types # Test 2: Check if the replace-content-within-marker utility exists within the 'utils' directory fd "replace-content-within-marker.ts" "replace-content-within-marker.js" ./utilsLength of output: 433
Script:
#!/bin/bash # Description: Verify the existence of the imported files across the repository # Test 1: Search for 'types.ts' or 'types.js' anywhere in the repository fd --extension ts --extension js "types\.(ts|js)" # Test 2: Search for 'replace-content-within-marker.ts' or 'replace-content-within-marker.js' anywhere in the repository fd --extension ts --extension js "replace-content-within-marker\.(ts|js)"Length of output: 741
packages/packem/src/types.ts (1)
32-32: LGTM: New import for FileCache type.The new import for the
FileCachetype is correctly added as a type import, which is good for keeping the runtime bundle size small.packages/packem/package.json (3)
141-150: LGTM: New export for./builder/typedocadded.The new export for
./builder/typedocis correctly structured and consistent with other exports in the file. This addition aligns with the introduction of the newbuilderfunction mentioned in the AI summary.Please ensure that the referenced files (
index.d.mts,index.mjs,index.d.cts,index.cjs) exist in thedist/builder/typedoc/directory after building the project.
182-183: LGTM: NewtypesVersionsentry forbuilder/typedocadded.The new entry in
typesVersionsforbuilder/typedoccorrectly corresponds to the new export and follows the established pattern in this section.
Line range hint
1-377: Overall: Changes align with PR objectives and enhance TypeDoc functionality.The modifications to
package.jsonsuccessfully introduce new TypeDoc-related functionality and maintain the file's structure. The changes are consistent with the PR objectives and the AI-generated summary. Minor suggestions have been made regarding version specifications for better flexibility and easier updates.packages/packem/README.md (7)
Line range hint
1-58: Excellent enhancement of the introduction and features section!The expanded introduction and detailed features list provide a comprehensive overview of Packem's capabilities. This aligns well with the PR objectives and offers valuable information to potential users.
Line range hint
181-420: Comprehensive usage guide with room for improvementThe usage section provides detailed instructions and covers advanced features, which is excellent. However, the "Building Module Workers" section is marked as WIP (Work in Progress).
Could you provide more details or a timeline for completing the "Building Module Workers" section? This would help users understand when to expect this feature.
Line range hint
421-453: Clear explanation of aliases usage and limitationsThe Aliases section provides valuable information on configuring and using aliases, including important limitations regarding type declarations. This addition enhances the documentation's completeness.
Line range hint
454-495: Comprehensive coverage of ESM ⇄ CJS interoperabilityThe explanation of ESM and CJS interoperability is clear and informative. The inclusion of configuration instructions for enabling CJS interop features is particularly helpful for users dealing with mixed module systems.
Line range hint
496-503: Helpful validation information and resourcesThe Validating section provides useful information about package.json validation and includes links to external validation tools. This helps users ensure their package configurations are correct and complete.
Line range hint
472-538: Excellent addition of TypeDoc documentationThe new TypeDoc section provides clear installation instructions and usage information for generating API documentation. This addition enhances the project's documentation capabilities and aligns well with the PR objectives.
Line range hint
1-586: Overall excellent enhancement of the READMEThe changes to the README.md file significantly improve the documentation for the Visulima Packem project. The additions and modifications provide comprehensive information about new features, usage instructions, and advanced capabilities. This aligns well with the PR objectives and enhances the overall user experience.
A few minor suggestions for improvement:
- Consider adding more details or a timeline for the "Building Module Workers" section.
- Enhance the TypeScript example in the project preparation section with an explanation of the
typeVersionsfield.Great job on improving the documentation!
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
…em [1.1.0](https://github.com/visulima/packem/compare/@visulima/[email protected]...@visulima/[email protected]) (2024-10-22) ### Features * add command to install optional features & builder option ([#40](#40)) ([2d64af9](2d64af9))
|
This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. |
Summary by CodeRabbit
Release Notes
New Features
README.mdwith detailed documentation on features, installation, and usage ofVisulima Packem.package.jsonfields.Bug Fixes
Documentation
Chores
package.json.