Conversation
|
Important Review skippedReview was skipped due to path filters ⛔ Files ignored due to path filters (2)
CodeRabbit blocks several paths by default. You can override this behavior by explicitly including those paths in the path filters. For example, including You can disable this status message by setting the WalkthroughThe pull request introduces numerous updates and new files related to a compression plugin implementation. Modifications include adjustments to the GitHub build workflow (with command formatting and version updates for actions), and the addition of new plugin components across several directories. These encompass Rust source files, TypeScript interfaces, NPM package configurations for multiple target platforms, playground resources for a Farm+React setup, and supporting scripts. Additionally, several legacy rustfmt.toml configuration files have been removed from various submodules. Changes
Sequence Diagram(s)sequenceDiagram
participant Farm as Farm Build Process
participant Plugin as FarmfePluginCompress
participant Utils as Compression Utils
participant FS as File System
Farm->>Plugin: Initiate compression on assets
Plugin->>Utils: Call compress_buffer(buffer, algorithm, level)
Utils-->>Plugin: Return compressed data
Plugin->>FS: Replace original files (if deleteOriginFile enabled)
Plugin->>Farm: Report compression summary and stats
Poem
🪧 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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (15)
rust-plugins/compress/src/lib.rs (2)
84-85: Consider compiling the regex once earlier.
Regex compilation occurs during each finalize pass, which might be repetitive if called multiple times. Compiling the regex in the constructor could improve performance.
130-140: Provide configurable logging.
The color-coded log is helpful, but some environments may not support ANSI escape sequences. Consider adding a configuration option to disable or alter log formatting for different environments.rust-plugins/compress/scripts/index.d.ts (1)
1-3: Validate or document default option values.
Although this declaration is minimal, ensuring thatbinPath()clearly documents or enforces valid defaults forIPluginOptionscan prevent misuse.Do you want to add additional type checks or JSDoc comments explaining the default behavior?
rust-plugins/compress/options.d.ts (1)
1-34: LGTM! Well-structured interface with clear documentation.The interface is well-documented with JSDoc comments, sensible defaults, and clear type definitions. The use of a regex string instead of a RegExp object for the filter option is a good choice to avoid deserialization issues.
Consider adding common image formats to the default filter pattern since they often benefit from compression:
- * @default '\\.(js|mjs|json|css|html)$' + * @default '\\.(js|mjs|json|css|html|png|jpg|jpeg|gif|svg)$'rust-plugins/compress/playground/src/main.tsx (2)
7-7: Remove debug console.log statement.The console.log statement appears to be debug code that should be removed before production.
- console.log("rendering Main component")
11-16: Add security attributes to external links.External links opening in new tabs should include
rel="noopener noreferrer"to prevent potential security vulnerabilities.- <a href="https://farmfe.org/" target="_blank"> + <a href="https://farmfe.org/" target="_blank" rel="noopener noreferrer"> - <a href="https://react.dev" target="_blank"> + <a href="https://react.dev" target="_blank" rel="noopener noreferrer">rust-plugins/compress/src/utils.rs (2)
77-88: Document magic numbers in brotli_compress function.The function uses magic numbers (4096, 22) without explanation. These parameters affect compression performance and should be documented.
+ // Buffer size of 4096 bytes and window size of 22 (4MB) for optimal compression let mut encoder = brotli::CompressorWriter::new(Vec::new(), 4096, level, 22);
23-88: Reduce error handling duplication.The error mapping code is duplicated across all compression functions. Consider extracting it into a helper function.
fn map_compression_error(algorithm: &str, error: impl std::error::Error + 'static) -> CompilationError { CompilationError::GenerateResourcesError { name: algorithm.to_string(), ty: ResourcePotType::Custom(algorithm.to_string()), source: Some(Box::new(error)), } }rust-plugins/compress/scripts/index.js (1)
27-122: Reduce code duplication in platform-specific logic.The binary path resolution logic is repeated for each platform/architecture combination. Consider extracting this into a helper function.
function resolveBinaryPath(platform, arch, variant = '') { const platformPath = `${platform}-${arch}${variant}`; const localPath = join(currentDir, `../npm/${platformPath}/index.farm`); return existsSync(localPath) ? localPath : require.resolve(`@farmfe/plugin-compress-${platformPath}`); }rust-plugins/compress/.cargo/config.toml (1)
16-25: AArch64 Linux MUSL Configuration Caution
The[target.aarch64-unknown-linux-musl]section sets a MUSL linker and applies rustflags including"-C", "target-feature=-crt-static". Since MUSL is typically associated with fully static linking, please double-check that disabling CRT static linking is intentional for your deployment scenario.rust-plugins/compress/.github/workflows/ci.yaml (2)
11-29: Artifact Handling in Test Artifacts Job
The job downloads artifacts to/tmp/artifactsand then iterates over a set of ABI configurations to move plugin files to./npm/${abi}and verify the existence ofindex.farm. Consider checking or creating the target directories before the move to avoid potential errors if they do not already exist.
36-36: Newline at EOF
A newline at the end of the file is missing. Please add one to comply with YAML formatting best practices.🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 36-36: no new line character at the end of file
(new-line-at-end-of-file)
rust-plugins/compress/.github/workflows/release.yml (2)
31-37: Artifact Relocation in Release Job
The for-loop moves artifacts into the expected npm directories and validates the presence ofindex.farm. Similar to the CI workflow, consider ensuring that the destination directories exist to avoid move errors.
42-42: Newline at EOF
Please add a newline at the end of the file to meet YAML formatting requirements.rust-plugins/compress/pnpm-workspace.yaml (1)
1-1: Remove Trailing Spaces
The static analysis tool detected trailing spaces on line 1. Please remove any trailing whitespace to maintain clean YAML formatting.-packages: +packages:🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 1-1: trailing spaces
(trailing-spaces)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (7)
Cargo.lockis excluded by!**/*.lockpnpm-lock.yamlis excluded by!**/pnpm-lock.yamlrust-plugins/compress/Cargo.lockis excluded by!**/*.lockrust-plugins/compress/playground/public/favicon.icois excluded by!**/*.icorust-plugins/compress/playground/src/assets/logo.pngis excluded by!**/*.pngrust-plugins/compress/playground/src/assets/react.svgis excluded by!**/*.svgrust-plugins/compress/pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (48)
.github/workflows/build.yaml(2 hunks)rust-plugins/compress/.cargo/config.toml(1 hunks)rust-plugins/compress/.github/workflows/build.yaml(1 hunks)rust-plugins/compress/.github/workflows/ci.yaml(1 hunks)rust-plugins/compress/.github/workflows/release.yml(1 hunks)rust-plugins/compress/.gitignore(1 hunks)rust-plugins/compress/Cargo.toml(1 hunks)rust-plugins/compress/npm/darwin-arm64/README.md(1 hunks)rust-plugins/compress/npm/darwin-arm64/package.json(1 hunks)rust-plugins/compress/npm/darwin-x64/README.md(1 hunks)rust-plugins/compress/npm/darwin-x64/package.json(1 hunks)rust-plugins/compress/npm/linux-arm64-gnu/README.md(1 hunks)rust-plugins/compress/npm/linux-arm64-gnu/package.json(1 hunks)rust-plugins/compress/npm/linux-arm64-musl/README.md(1 hunks)rust-plugins/compress/npm/linux-arm64-musl/package.json(1 hunks)rust-plugins/compress/npm/linux-x64-gnu/README.md(1 hunks)rust-plugins/compress/npm/linux-x64-gnu/package.json(1 hunks)rust-plugins/compress/npm/linux-x64-musl/README.md(1 hunks)rust-plugins/compress/npm/linux-x64-musl/package.json(1 hunks)rust-plugins/compress/npm/win32-arm64-msvc/README.md(1 hunks)rust-plugins/compress/npm/win32-arm64-msvc/package.json(1 hunks)rust-plugins/compress/npm/win32-ia32-msvc/README.md(1 hunks)rust-plugins/compress/npm/win32-ia32-msvc/package.json(1 hunks)rust-plugins/compress/npm/win32-x64-msvc/README.md(1 hunks)rust-plugins/compress/npm/win32-x64-msvc/package.json(1 hunks)rust-plugins/compress/options.d.ts(1 hunks)rust-plugins/compress/package.json(1 hunks)rust-plugins/compress/playground/README.md(1 hunks)rust-plugins/compress/playground/farm.config.ts(1 hunks)rust-plugins/compress/playground/index.html(1 hunks)rust-plugins/compress/playground/index.js(1 hunks)rust-plugins/compress/playground/package.json(1 hunks)rust-plugins/compress/playground/src/index.css(1 hunks)rust-plugins/compress/playground/src/index.tsx(1 hunks)rust-plugins/compress/playground/src/main.css(1 hunks)rust-plugins/compress/playground/src/main.tsx(1 hunks)rust-plugins/compress/playground/src/typings.d.ts(1 hunks)rust-plugins/compress/playground/tsconfig.json(1 hunks)rust-plugins/compress/playground/tsconfig.node.json(1 hunks)rust-plugins/compress/pnpm-workspace.yaml(1 hunks)rust-plugins/compress/rust-toolchain.toml(1 hunks)rust-plugins/compress/rustfmt.toml(1 hunks)rust-plugins/compress/scripts/func.js(1 hunks)rust-plugins/compress/scripts/index.d.ts(1 hunks)rust-plugins/compress/scripts/index.js(1 hunks)rust-plugins/compress/scripts/watch.sh(1 hunks)rust-plugins/compress/src/lib.rs(1 hunks)rust-plugins/compress/src/utils.rs(1 hunks)
✅ Files skipped from review due to trivial changes (35)
- rust-plugins/compress/playground/index.js
- rust-plugins/compress/scripts/func.js
- rust-plugins/compress/scripts/watch.sh
- rust-plugins/compress/npm/win32-x64-msvc/README.md
- rust-plugins/compress/playground/tsconfig.node.json
- rust-plugins/compress/rustfmt.toml
- rust-plugins/compress/playground/src/index.tsx
- rust-plugins/compress/npm/linux-x64-musl/README.md
- rust-plugins/compress/npm/linux-x64-gnu/README.md
- rust-plugins/compress/npm/win32-ia32-msvc/README.md
- rust-plugins/compress/npm/win32-arm64-msvc/README.md
- rust-plugins/compress/npm/darwin-arm64/README.md
- rust-plugins/compress/rust-toolchain.toml
- rust-plugins/compress/npm/win32-arm64-msvc/package.json
- rust-plugins/compress/npm/darwin-x64/README.md
- rust-plugins/compress/npm/linux-arm64-musl/README.md
- rust-plugins/compress/.gitignore
- rust-plugins/compress/npm/linux-arm64-gnu/README.md
- rust-plugins/compress/npm/linux-arm64-gnu/package.json
- rust-plugins/compress/npm/linux-x64-musl/package.json
- rust-plugins/compress/playground/index.html
- rust-plugins/compress/npm/darwin-arm64/package.json
- rust-plugins/compress/npm/darwin-x64/package.json
- rust-plugins/compress/npm/linux-arm64-musl/package.json
- rust-plugins/compress/playground/src/typings.d.ts
- rust-plugins/compress/playground/tsconfig.json
- rust-plugins/compress/package.json
- rust-plugins/compress/npm/win32-ia32-msvc/package.json
- rust-plugins/compress/npm/linux-x64-gnu/package.json
- rust-plugins/compress/playground/README.md
- rust-plugins/compress/npm/win32-x64-msvc/package.json
- rust-plugins/compress/playground/src/index.css
- rust-plugins/compress/playground/package.json
- rust-plugins/compress/playground/src/main.css
- rust-plugins/compress/Cargo.toml
🧰 Additional context used
🪛 YAMLlint (1.35.1)
rust-plugins/compress/.github/workflows/release.yml
[error] 43-43: no new line character at the end of file
(new-line-at-end-of-file)
rust-plugins/compress/.github/workflows/ci.yaml
[error] 36-36: no new line character at the end of file
(new-line-at-end-of-file)
rust-plugins/compress/pnpm-workspace.yaml
[error] 1-1: trailing spaces
(trailing-spaces)
⏰ Context from checks skipped due to timeout of 90000ms (9)
- GitHub Check: call-rust-build / Build and Upload Artifacts - linux-arm64-gnu
- GitHub Check: call-rust-build / Build and Upload Artifacts - linux-arm64-musl
- GitHub Check: call-rust-build / Build and Upload Artifacts - win32-arm64-msvc
- GitHub Check: call-rust-build / Build and Upload Artifacts - win32-ia32-msvc
- GitHub Check: call-rust-build / Build and Upload Artifacts - darwin-x64
- GitHub Check: call-rust-build / Build and Upload Artifacts - darwin-arm64
- GitHub Check: call-rust-build / Build and Upload Artifacts - win32-x64-msvc
- GitHub Check: call-rust-build / Build and Upload Artifacts - linux-x64-musl
- GitHub Check: call-rust-build / Build and Upload Artifacts - linux-x64-gnu
🔇 Additional comments (48)
rust-plugins/compress/src/lib.rs (1)
91-96: Confirm safe parallel mutation.
Settingresource.emitted = truewhile traversingresources_mapin parallel may require careful synchronization ifresources_mapcan be accessed concurrently elsewhere. Make sure no data race can occur.rust-plugins/compress/playground/farm.config.ts (1)
16-20: Revisit compression threshold and level.
A threshold of 2048 and a level of 11 could offer significant compression but also increase CPU usage. Consider verifying performance metrics to ensure these parameters are optimal for your application’s size and use case.rust-plugins/compress/src/utils.rs (1)
90-96: Clarify extension naming for Deflate variants.Both DeflateRaw and Deflate use the same extension "deflate", which might cause confusion. Consider using distinct extensions or documenting the rationale.
Please confirm if using the same extension for both Deflate variants is intentional and won't cause issues with content encoding headers in HTTP responses.
rust-plugins/compress/.cargo/config.toml (7)
2-4: Network Configuration Verification
The[net]section enablesgit-fetch-with-clifor Git operations via the CLI. Confirm that this option meets your network fetch needs.
5-9: Global Build Settings Review
The[build]section setsrustdocflagsand a globalrustflagsfor shared generics. This configuration looks appropriate for generating docs and optimizing generic code sharing.
10-12: Target-Specific Flags for x86_64 Linux
The[target.x86_64-unknown-linux-gnu]configuration applies the SSE2 feature flag and the shared generics flag. This configuration is clear and should improve performance on x86_64 Linux targets.
13-15: AArch64 Linux GNU Linker Configuration
The[target.aarch64-unknown-linux-gnu]section correctly defines the linker to use (aarch64-linux-gnu-gcc). No concerns here.
26-29: Windows MSVC (x86_64) Configuration
For[target.x86_64-pc-windows-msvc], the configuration specifiesrust-lldas the linker with flags enforcing CRT static linking. This appears correct.
30-33: AArch64 Windows MSVC Configuration
Only a linker is specified for[target.aarch64-pc-windows-msvc]. If additional flags become necessary for this target in the future, consider adding them; otherwise, this minimal configuration is acceptable.
34-37: ARMv7 Linux GNUEABI Configuration
The[target.armv7-unknown-linux-gnueabi]section provides the appropriate linker and rustflags. This legacy target configuration meets standard requirements.rust-plugins/compress/.github/workflows/ci.yaml (4)
1-6: Workflow Trigger and Naming
The workflow is named "Test Plugin" and is set to trigger on pull requests to themainbranch. This configuration clearly targets CI validation of the plugin artifacts.
7-10: Chaining Build Workflows
Thecall-rust-buildjob references the shared build workflow (build.yaml), ensuring that the plugin build is executed prior to artifact testing.
29-33: Node.js Environment Setup
The workflow usesactions/setup-node@v3with Node.js version 18.x. This step ensures a consistent Node.js environment for subsequent dependency installation and build steps.
35-36: Running Build Example in Playground
The final step navigates into theplaygrounddirectory and runs dependency installation followed by a build. Verify that the playground is correctly configured and that its build script operates as expected.🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 36-36: no new line character at the end of file
(new-line-at-end-of-file)
rust-plugins/compress/.github/workflows/release.yml (8)
1-6: Release Workflow Trigger and Naming
The release workflow is set to trigger on pushes to themainbranch and is aptly named. The inclusion of concurrency helps avoid overlapping release runs.
7-8: Concurrency Configuration
Using a concurrency group based on the workflow and branch ensures isolated release runs. This design is effective in continuous deployment scenarios.
9-12: Pre-Release Build Dependency
Thecall-rust-buildjob is invoked prior to the release job, ensuring that the build artifacts are correctly prepared before proceeding with publishing.
13-17: Release Job Setup
The release job is configured with a dependency on the build job and is set to run on an Ubuntu environment. This clear delineation of steps supports a smooth release process.
18-20: Repository Checkout
The checkout step ensures that the repository’s code is available for the release process, which is a standard best practice.
21-25: Node.js Setup for Release
The workflow sets up Node.js (version 18.x) using the corresponding action, ensuring consistency with the build environment.
26-30: Artifact Download Step
Artifacts are downloaded from the build process into a temporary directory, aligning with the subsequent steps for artifact relocation.
38-42: Dependency Installation and Publishing
The workflow installs dependencies with PNPM and then configures npm authentication before runningnpm publish. Ensure that theNPM_TOKENsecret is correctly set and that robust error handling is in place for the publishing step.rust-plugins/compress/.github/workflows/build.yaml (13)
1-3: Compress Plugin Build Workflow Initialization
This workflow, invoked viaworkflow_call, is well-named for its role in building and uploading Rust bindings for the compress plugin. The trigger and metadata are clear.
4-10: Job and Matrix Strategy Configuration
The matrix strategy is effectively used to run builds across multiple operating systems and ABI targets. Ensure that all matrix configurations reflect your supported environments accurately.
11-20: Matrix Entry: Linux x86_64-gnu Build
The build command for thelinux-x64-gnutarget sets necessary environment commands and invokesnpm run buildwith clear target parameters.
21-29: Matrix Entry: Linux x86_64-musl Build
The configuration for the musl target is set up with careful environment resets and the appropriate build command. The formatting is consistent, which is beneficial for maintainability.
30-35: Matrix Entries for Windows and macOS Targets
For targets likewin32-x64-msvc,darwin-x64, anddarwin-arm64, the configurations omit custom build commands, implying defaults are acceptable. Confirm that these targets function properly with the default invocations.
35-44: Cross-compilation for Windows
The cross-compilation steps forwin32-ia32-msvcandwin32-arm64-msvcinclude custom build scripts that installcargo-xwinand configure environment variables. These detailed commands demonstrate careful consideration of cross-compilation challenges.
54-60: Matrix Entries for Linux ARM Targets
The ARM configurations for both MUSL and GNU targets include support for Zig (when enabled). Verify that the selected Zig version remains compatible with your toolchain.
72-77: Checkout and Caching Steps
The steps to checkout the repository usingactions/checkout@v3and caching rust artifacts withSwatinem/rust-cache@v2are correctly implemented to speed up the build process.
77-81: Node.js Setup and Dependency Installation
Setting up Node.js (version 18) and installing PNPM dependencies with a frozen lockfile ensures a reproducible build environment.
82-83: Adding Rust Target
The conditional addition of the Rust target based on the matrix settings ensures that the build environment is correctly configured for each target architecture.
84-91: Cross-compilation Tools Setup
The steps to configure OSXCROSS and Zig are conditionally executed and support cross-compiling for macOS and specific ARM targets. This approach enhances the build’s portability.
92-98: Docker Build Command Update
The Docker build step now includes the--user 0:0option on the root build workflow, which ensures proper file ownership and user context within the container. This is an important update for permission consistency.
99-107: Fallback Build and Artifact Upload Steps
The workflow provides conditional fallback build commands when Docker is not used, followed by detailed artifact upload steps for various plugin components. This ensures comprehensive coverage of build outputs..github/workflows/build.yaml (13)
1-3: Global Build Workflow Initialization
This top-level build workflow is designed for building Rust bindings and uploading artifacts. Its use ofworkflow_callhighlights its role as a central component in the CI/CD pipeline.
4-10: Job and Matrix Strategy Setup
The overall job configuration and matrix strategy span multiple OS and ABI targets, with caching keyed by ABI. This granular approach is effective for maintaining build consistency across environments.
11-20: Linux x86_64-gnu Build Command
The build command for the GNU target correctly sets the safe directory, resets environment variables, and invokespnpmwith the proper target and ABI flags.
21-29: Linux x86_64-musl Build Command Formatting
The musl target’s build command is now correctly formatted without extraneous spaces. This small but important cleanup improves readability and consistency.
30-35: Matrix Entries for Windows and macOS Targets
The configurations for Windows and macOS targets are declared without extra build commands, implying that default procedures are adequate. Ensure that these targets are thoroughly tested in your CI environment.
36-44: Windows Cross-compilation Setup
Custom build steps for cross-compiling to Windows (both ia32 and arm64) are included, featuring environment variable setups and the installation ofcargo-xwin. These details accommodate the complexity of Windows builds effectively.
54-62: Linux ARM Target Configuration with Zig
The ARM target configurations are clearly defined, and the optional support for Zig is a valuable addition for cross-compilation. Verify that Zig’s version remains consistent with your development needs.
64-69: Repository Checkout and Artifact Caching
Using a shallow checkout and caching Rust artifacts are both effective strategies for speeding up the build process while maintaining access to necessary source code details.🧰 Tools
🪛 actionlint (1.7.4)
64-64: the runner of "actions/checkout@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
70-77: Node.js Environment and Dependency Management
The explicit setup of Node.js along with the installation of PNPM dependencies using a frozen lockfile guarantees reproducibility. This step adheres to best practices.🧰 Tools
🪛 actionlint (1.7.4)
75-75: the runner of "actions/setup-node@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
78-83: Adding Rust Target
The step to add the appropriate Rust target, when specified in the matrix, is conditionally executed and helps ensure the toolchain is fully prepared for the build.
84-91: Cross-Compilation Tool Setup
The conditional setup for OSXCROSS and Zig facilitates cross-compiling to macOS and ARM targets, respectively. This enhances the workflow’s versatility.🧰 Tools
🪛 actionlint (1.7.4)
84-84: property "osxcross" is not defined in object type {abi: string; build: string; docker: string; os: string; target: string; zig: bool}
(expression)
92-98: Docker Build Command Update
The Docker run step now includes the--user 0:0option, ensuring that the build run inside Docker uses the root user correctly. This update is critical for handling file permissions in containerized environments.
99-107: Fallback Build and Artifact Upload Steps
The later steps conditionally trigger alternative build commands when Docker is not used and comprehensively upload artifacts for various plugin components. This robust handling supports modular artifact management for releases.
…gins into feat/plugin-compress
|
@CCherry07 I have reverted the changes on CI checks, all changes in |
3 participants
Summary by CodeRabbit
New Features
Documentation
Chores