Gulp to vite #2873
Gulp to vite #2873
Conversation
Code Review: Gulp to Vite MigrationThis is a substantial and well-executed migration from Gulp to Vite. The implementation demonstrates good understanding of both build systems and WordPress integration. Here's my detailed feedback: ✅ Strengths
🔴 Critical Issues1. Debug Logging Left in Production Code (
|
PR Review: Gulp to Vite MigrationThank you for this comprehensive migration! The overall implementation is well-executed and follows Vite best practices. However, there are some critical issues that need to be addressed before merging. 🚨 Critical Issues (Must Fix)1. Performance Regression: Legacy Bundle Forced on All UsersIn dt_theme_enqueue_script( 'site-polyfills', 'dt-assets/build/js/polyfills-legacy.min.js', array( 'jquery' ), true );
dt_theme_enqueue_script( 'site-js', 'dt-assets/build/js/scripts-legacy.min.js', array( 'jquery', 'site-polyfills' ), true );This significantly increases bundle size for modern browsers unnecessarily. The correct approach is:
Impact: Every user downloads the larger transpiled/polyfilled bundle instead of the optimized modern version. Suggested Fix: // Modern browsers
dt_theme_enqueue_script( 'site-js', 'dt-assets/build/js/scripts.min.js', array( 'jquery' ), true );
wp_script_add_data( 'site-js', 'type', 'module' );
// Legacy browsers (IE11, old Safari, etc.)
dt_theme_enqueue_script( 'site-polyfills-legacy', 'dt-assets/build/js/polyfills-legacy.min.js', array( 'jquery' ), true );
wp_script_add_data( 'site-polyfills-legacy', 'nomodule', true );
dt_theme_enqueue_script( 'site-js-legacy', 'dt-assets/build/js/scripts-legacy.min.js', array( 'jquery', 'site-polyfills-legacy' ), true );
wp_script_add_data( 'site-js-legacy', 'nomodule', true );Note: WordPress doesn't natively support 2. Debug Logging Left in Production Code
dt_write_log( 'serving dt-assets from vite' );
dt_write_log( 'serving dt-assets from static' );Action: Remove these before merging. 3. Composer Autoload in Wrong Location
if ( file_exists( get_template_directory() . '/vendor/autoload.php' ) ) {
require_once get_template_directory() . '/vendor/autoload.php';
}Issue: This should be loaded once in Action: Move this to
|
Pull Request Review: Gulp to Vite MigrationExecutive SummaryThis PR successfully migrates the build system from Gulp to Vite, bringing modern tooling, faster build times (~50% faster), and Hot Module Replacement (HMR) for CSS development. The implementation is well-structured and maintains feature parity with the previous Gulp setup while following modern best practices. Recommendation: ✅ Approve with minor observations noted below 1. Code Quality Analysisvite.config.jsStrengths:
Observations:
package.json ChangesStrengths:
postcss.config.mjsExcellent: Simple, clean PostCSS config with autoprefixer and cssnano. Uses 2. Build Configuration AnalysisSCSS Compilation ✅
JS Bundling ✅
Web Components ✅
Legacy Browser Support ✅
Output Paths ✅All paths match WordPress expectations:
3. Security AnalysisDependency Versions ✅
WordPress Integration Security ✅functions.php (lines 15-17): if ( file_exists( get_template_directory() . '/vendor/autoload.php' ) ) {
require_once get_template_directory() . '/vendor/autoload.php';
}
HMR Security ✅enqueue-scripts.php:
SSL Certificate Handling
|
| Category | Status | Notes |
|---|---|---|
| Code Quality | ✅ Excellent | Clean, well-commented |
| Build Config | ✅ Complete | All features migrated |
| Security | ✅ Good | No vulnerabilities |
| Performance | ✅ Improved | 50% faster, HMR enabled |
| Breaking Changes | IE 11 dropped, new workflow | |
| Best Practices | ✅ Strong | Modern patterns |
| WordPress Integration | ✅ Excellent | Proper fallbacks |
| Documentation | ✅ Comprehensive | CONTRIBUTING.md updated |
Overall Assessment: 9/10
This is an excellent implementation that successfully modernizes the build system while maintaining backward compatibility and following WordPress best practices. The code quality is high, documentation is thorough, and the developer experience improvements (HMR, faster builds) are significant.
Recommendation: Approve and merge. Post-merge, communicate IE 11 support drop and new build commands to team.
Great work! 🎉
1 participant
Replaces gulp build system with Vite. This cuts the build time down to half or less. It adds HMR (hot module replacement) for development of styles where any change to the SCSS will immediately re-compile and swap itself into your browser (in maybe 2-5 seconds, based on limited testing).
I added developer instructions in the contributing.md file for how to utilize HMR when working on styles. However, the standard
npm run buildcan still be used to just rebuild the production files and you can copy them to your site however you normally did in the past.Here is a final summary I got from AI as I was finishing it all (before I removed all the gulp files/dependencies):
AI summary and evaluation
I have completed a thorough review of the Gulp to Vite replacement. The migration is largely successful and covers all primary features of the legacy build system.
Key Findings & Verification
style.min.css,scripts.min.js, etc.) and paths match the existing theme expectations, ensuring WordPress compatibility.dt-assets/functions/enqueue-scripts.phpis well-integrated with Vite usingvite-for-wp, supporting both Hot Module Replacement (HMR) during development and optimized static assets in production.@vitejs/plugin-legacyis configured to provide polyfills and legacy bundles for older browsers, mirroring the previous Babel/Uglify workflow.touchcommand has been effectively replaced by WordPress'sfilemtimein the enqueue helper, ensuring cache busting works correctly without explicit file "touching" in the build process.Overall, the implementation is solid and follows modern best practices for WordPress frontend development.