Feat: import timings and bundle size analysis #2114
Conversation
🦋 Changeset detectedLatest commit: cf88632 The changes in this PR will be included in the next version bump. Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
WalkthroughThis change set introduces import timing measurement and bundle size analysis capabilities to the project. It adds a new hidden CLI command, ✨ 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:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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: 0
🧹 Nitpick comments (7)
packages/cli-v3/src/utilities/analyze.ts (4)
154-154: Consider making the slow import threshold configurable.The 1-second threshold for slow imports is hardcoded. Different projects might have different performance requirements.
Consider accepting the threshold as a parameter:
-export function printWarnings(workerManifest: WorkerManifest) { +export function printWarnings(workerManifest: WorkerManifest, slowImportThreshold = 1000) {And then:
- if (timing > 1000) { + if (timing > slowImportThreshold) {
298-310: Consider making bundle size thresholds configurable.The color-coding thresholds for bundle sizes (5MB for error, 1MB for warning) are hardcoded. These might need adjustment based on project requirements.
Consider adding threshold parameters or extracting them as constants:
+const BUNDLE_SIZE_ERROR_THRESHOLD = 5 * 1024 * 1024; // 5MB +const BUNDLE_SIZE_WARNING_THRESHOLD = 1024 * 1024; // 1MB + function formatSizeColored(bytes: number, withBraces = false): string { let str: string; - if (bytes > 5 * 1024 * 1024) { + if (bytes > BUNDLE_SIZE_ERROR_THRESHOLD) {
209-212: Improve regex pattern robustness.The regex pattern for normalizing paths could be more specific to avoid potential false matches.
Consider using a more specific pattern:
- return path.replace(/(^|\/).trigger\/tmp\/build-[^/]+\//, ""); + return path.replace(/(?:^|\/)\.trigger\/tmp\/build-[a-zA-Z0-9_-]+\//g, "");This ensures:
- Proper escaping of the dot in
.trigger- More specific character class for build hash
- Global flag in case of multiple occurrences
318-320: Clarify undefined timing display.When timing is undefined, displaying "?ms" might be confusing. Consider a clearer message.
if (typeof timing !== "number") { - str = "?ms"; + str = "N/A"; return withBraces ? chalkGreen(`(${str})`) : chalkGreen(str); }packages/cli-v3/src/commands/analyze.ts (3)
24-24: Document why the command is hidden.The
analyzecommand is marked as hidden. Consider adding a comment explaining why this is a hidden command (e.g., experimental feature, internal use only, etc.).+ // Hidden command for internal debugging and advanced users .command("analyze [dir]", { hidden: true })
54-62: Enhance error messages with actionable instructions.The error messages could be more helpful by providing specific instructions on how to generate the required files.
if (!fs.existsSync(metafilePath)) { logger.error(`Could not find metafile.json in ${targetDir}`); - logger.info("Make sure you have built your project and metafile.json exists."); + logger.info("To generate metafile.json, run 'trigger.dev dev --analyze' or ensure your build process creates this file."); return; } if (!fs.existsSync(manifestPath)) { logger.error(`Could not find index.json (worker manifest) in ${targetDir}`); - logger.info("Make sure you have built your project and index.json exists."); + logger.info("The worker manifest (index.json) is generated during the build process. Run 'trigger.dev dev' to create it."); return; }
150-150: Move type export to the top with other imports.The
Metafiletype export is at the bottom of the file. For better code organization, consider moving it near the top with other type definitions.Move line 150 to after line 20:
type AnalyzeOptions = z.infer<typeof AnalyzeOptions>; +type Metafile = z.infer<typeof MetafileSchema>;And remove it from line 150.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (15)
.changeset/late-dancers-smile.md(1 hunks).cursorignore(1 hunks)packages/cli-v3/src/cli/index.ts(2 hunks)packages/cli-v3/src/commands/analyze.ts(1 hunks)packages/cli-v3/src/commands/dev.ts(3 hunks)packages/cli-v3/src/dev/backgroundWorker.ts(2 hunks)packages/cli-v3/src/dev/devOutput.ts(2 hunks)packages/cli-v3/src/dev/devSession.ts(3 hunks)packages/cli-v3/src/dev/devSupervisor.ts(3 hunks)packages/cli-v3/src/dev/workerRuntime.ts(1 hunks)packages/cli-v3/src/entryPoints/dev-index-worker.ts(2 hunks)packages/cli-v3/src/entryPoints/managed-index-worker.ts(2 hunks)packages/cli-v3/src/indexing/registerResources.ts(2 hunks)packages/cli-v3/src/utilities/analyze.ts(1 hunks)packages/core/src/v3/schemas/build.ts(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (7)
packages/cli-v3/src/cli/index.ts (1)
packages/cli-v3/src/commands/analyze.ts (1)
configureAnalyzeCommand(22-38)
packages/cli-v3/src/dev/devSession.ts (1)
packages/cli-v3/src/build/extensions.ts (1)
notifyExtensionOnBuildComplete(31-53)
packages/cli-v3/src/commands/analyze.ts (2)
packages/cli-v3/src/cli/common.ts (2)
handleTelemetry(39-49)wrapCommandAction(53-103)packages/cli-v3/src/utilities/analyze.ts (2)
printBundleTree(27-104)printBundleSummaryTable(106-140)
packages/cli-v3/src/entryPoints/dev-index-worker.ts (1)
packages/cli-v3/src/indexing/registerResources.ts (1)
registerResources(4-42)
packages/cli-v3/src/entryPoints/managed-index-worker.ts (1)
packages/cli-v3/src/indexing/registerResources.ts (1)
registerResources(4-42)
packages/cli-v3/src/dev/devOutput.ts (1)
packages/cli-v3/src/utilities/analyze.ts (1)
analyzeWorker(8-25)
packages/cli-v3/src/indexing/registerResources.ts (2)
packages/core/src/v3/schemas/build.ts (2)
ImportTaskFileErrors(122-122)ImportTaskFileErrors(124-124)packages/core/src/v3/resource-catalog-api.ts (1)
resourceCatalog(5-5)
⏰ Context from checks skipped due to timeout of 90000ms (9)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (6, 10)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (4, 10)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (6, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (5, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (2, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (1, 8)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - pnpm)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - npm)
- GitHub Check: typecheck / typecheck
🔇 Additional comments (28)
.cursorignore (1)
8-8: LGTM! Good security practice.Adding
.envto the ignore list is appropriate to prevent sensitive environment variables from being processed by the cursor tool..changeset/late-dancers-smile.md (1)
1-7: LGTM! Changeset correctly documents the feature.The changeset properly specifies patch-level changes for both affected packages and accurately describes the new import timing and bundle size analysis functionality.
packages/cli-v3/src/cli/index.ts (2)
9-9: LGTM! Import follows established patterns.The import statement for
configureAnalyzeCommandis correctly placed and follows the existing pattern of other command imports.
38-38: LGTM! Command registration follows established patterns.The
configureAnalyzeCommandcall is properly placed alongside other command configurations and maintains consistency with the existing codebase structure.packages/core/src/v3/schemas/build.ts (1)
93-93: LGTM! Schema extension is well-designed.The addition of the optional
timingsproperty usingz.record(z.number())is appropriate for storing timing data as string-to-number mappings. Making it optional ensures backward compatibility while enabling the new import timing analysis feature.packages/cli-v3/src/entryPoints/managed-index-worker.ts (4)
89-89: LGTM! Timings collection properly integrated.The destructuring of
timingsfromregisterResourcescorrectly aligns with the updated function signature that now returns import timing data alongside import errors.
96-96: LGTM! Timings properly returned from bootstrap.The
timingsdata is correctly included in the bootstrap function's return value, maintaining consistency with the function's purpose of collecting initialization data.
100-100: LGTM! Consistent destructuring pattern.The top-level destructuring correctly extracts the
timingsvalue from the bootstrap function, maintaining consistency with other extracted values.
162-162: LGTM! Timings properly propagated to manifest.The
timingsdata is correctly included in theINDEX_COMPLETEmessage manifest, enabling downstream analysis tools to access import timing information.packages/cli-v3/src/entryPoints/dev-index-worker.ts (1)
89-89: LGTM! Consistent timing collection implementation.The timing collection and propagation logic is correctly implemented and consistent with the pattern used in
managed-index-worker.ts. This ensures uniform behavior across both dev and managed worker entry points.Also applies to: 96-96, 100-100, 162-162
packages/cli-v3/src/dev/devOutput.ts (2)
29-29: LGTM! Proper import of analysis functionality.The import of
analyzeWorkerfrom the utilities module correctly brings in the bundle analysis functionality for integration into the dev workflow.
75-75: LGTM! Well-positioned analysis call.The
analyzeWorkercall is correctly placed in thebackgroundWorkerInitializedevent handler and properly uses the command-line options (analyzeanddisableWarnings) to control analysis behavior. This ensures analysis runs after successful worker initialization when requested.packages/cli-v3/src/dev/backgroundWorker.ts (2)
8-8: LGTM! Proper type import for metafile support.The import of the
Metafiletype from esbuild is correct and necessary for the constructor parameter addition.
23-23: LGTM! Clean constructor extension for metafile support.The addition of the
metafileparameter to the constructor properly extends theBackgroundWorkerclass to store esbuild metafile data, enabling the bundle analysis functionality. The parameter is correctly positioned and typed.packages/cli-v3/src/dev/workerRuntime.ts (2)
5-5: LGTM: Clean import addition.The import for
Metafilefrom esbuild is correctly added to support the new parameter in the interface.
9-9: LGTM: Well-structured interface update.The method signature update correctly adds the
metafileparameter with proper typing. The parameter placement betweenmanifestandstopis logical and maintains good parameter ordering.packages/cli-v3/src/commands/dev.ts (3)
2-2: LGTM: Proper import for CommandOption.The import correctly adds
Option as CommandOptionto support the new hidden CLI options being added.
27-28: LGTM: Well-structured schema additions.The new boolean options
analyzeanddisableWarningsfollow the existing schema pattern with appropriate default values offalse.
59-62: LGTM: Appropriately hidden CLI options.The hidden CLI options are well-implemented:
- Uses
CommandOptionclass correctly for advanced option configuration- Options are appropriately hidden from help output as these appear to be internal/debug features
- Clear descriptions for both analyze and disable-warnings functionality
packages/cli-v3/src/dev/devSession.ts (4)
27-28: LGTM: Appropriate imports for file operations.The imports for
writeJSONFileandjoinare correctly added to support writing the metafile to disk.
109-112: LGTM: Proper metafile persistence.The metafile is correctly written to disk using proper path joining. The file is placed in the appropriate directory (workerDir or destination) as
metafile.json, which follows a logical naming convention.
119-123: LGTM: Correct worker initialization update.The
initializeWorkercall is properly updated to include thebundle.metafileparameter, maintaining the correct parameter order as defined in the interface.
174-175: LGTM: Improved debug logging.The debug log message is more descriptive and helpful for understanding the first bundle behavior.
packages/cli-v3/src/indexing/registerResources.ts (4)
6-6: LGTM: Proper return type update.The return type correctly reflects the new structure that includes both
importErrorsandtimings, maintaining type safety.
8-8: LGTM: Appropriate timing storage initialization.The
timingsrecord is properly initialized to store timing data keyed by file entry names.
14-17: LGTM: Accurate performance timing implementation.The timing measurement is well-implemented:
- Uses
performance.now()for high-precision timing- Captures timing around the actual import operation
- Stores timing data with the file entry as the key
- Timing is recorded regardless of import success/failure, providing complete data for analysis
41-41: LGTM: Consistent return structure.The return statement correctly provides both
importErrorsandtimings, matching the updated function signature.packages/cli-v3/src/dev/devSupervisor.ts (1)
27-27: LGTM! Proper integration of esbuild metafile.The changes correctly propagate the esbuild
metafilethrough the worker initialization flow, enabling the new bundle analysis features. The type import and method signature updates are consistent and well-structured.Also applies to: 116-120, 130-130
The dev command will now print warnings for slow imports with helpful tips to remedy this.
New flags:
--analyzefor detailed output--disable-warningsto stop printing warnings