Conversation
WalkthroughThis pull request updates development and production dependencies across the project, introduces a node_modules guard in the test framework's directory traversal, strengthens a generic type constraint, and migrates HTML minification from html-minifier to html-minifier-next with an async conversion. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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. Comment |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
local_modules/simple-test-framework/package.json (1)
13-15: Dev TypeScript tooling bumps look fineUpdating
ts-nodeandtypescripthere is reasonable for this local-only package; no issues from a code-review perspective. If you ever want to simplify tooling, you could align this package’s TypeScript version with the root one to avoid maintaining two versions, but that’s optional.Please just confirm your test suite still runs cleanly with this combo in your local environment.
src/minify/minifyContents.ts (1)
20-22: Redundantawaitin returnSince
minifyContentsis alreadyasyncandminifyHtmlreturns aPromise<string>, you can simplify this branch toreturn minifyHtml(contents);without changing behaviour.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (5)
local_modules/simple-test-framework/package.json(1 hunks)local_modules/simple-test-framework/src/index.ts(1 hunks)package.json(1 hunks)src/core/utils/iterateKeys.ts(1 hunks)src/minify/minifyContents.ts(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Codacy Static Code Analysis
🔇 Additional comments (4)
local_modules/simple-test-framework/src/index.ts (1)
125-127: node_modules guard in recursive test discovery looks goodSkipping
node_modulesduring recursion is a solid safeguard and should noticeably improve performance and avoid spurious test files under dependencies.src/minify/minifyContents.ts (1)
1-12: html-minifier-next migration and options look reasonableThe switch to
html-minifier-nextwith an asyncminifyHtmlwrapper and the current option set looks consistent. The explicitremoveScriptTypeAttributes: falsewill preserve scripttypeattributes, which may differ from prior defaults, so it’s worth confirming this matches the behaviour you expect.Please double-check against the
html-minifier-nextdocs/release notes that these options mirror (or intentionally adjust) your previous minification behaviour.package.json (1)
58-60: Dependency and Node engine updates align with code changesThe move to
html-minifier-next, the Twig and TypeScript/typing bumps, and raisingengines.nodeto>=18.0.0all look consistent with the rest of the PR (async minification, newer TS, etc.). No obvious issues from a code perspective.Please run your test/build pipeline under Node 18+ to confirm there are no subtle behaviour changes from the new
html-minifier-next,twig, or the updated@types/node/TypeScript combo.Also applies to: 62-64, 66-66, 69-69
src/core/utils/iterateKeys.ts (1)
1-9: Stronger generic constraint is safe across all call sitesConfining
Ttoextends objectaligns the type system with the runtime assumption (Object.keys(data)) and improves compile-time safety. All call sites in the codebase already pass objects, so this is not a breaking change.
This pull request updates dependencies, improves compatibility, and refines functionality in both the main project and the local test framework. The most important changes are grouped below:
Dependency and Compatibility Updates:
html-minifiertohtml-minifier-nextand changed all related imports and type definitions inpackage.json,src/minify/minifyContents.ts, anddevDependenciesto improve HTML minification performance and compatibility. [1] [2]typescript(to v5.9.3 in the main project and v3.9.10 in the test framework),@types/node, and@types/twigfor broader compatibility and improved type safety. Also raised the minimum required Node.js version to 18. [1] [2]Functionality Improvements:
minifyHtmlinsrc/minify/minifyContents.tsto be asynchronous and use the newminifyAPI fromhtml-minifier-next, ensuring better performance and future-proofing. [1] [2]findAndRunTestsfunction in the simple test framework to skipnode_modulesdirectories, preventing unnecessary traversal and speeding up test discovery.Type Safety Enhancement:
iterateKeysinsrc/core/utils/iterateKeys.tsto ensure that only objects are accepted, increasing type safety and preventing runtime errors.Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.