refactor: track bundle size and gzip size in raw bytes #259
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- Add `originalBytes` and `obfuscatedBytes` to store raw byte values. - Update `calculateBundleSize` to return both formatted sizes and raw byte data. - Improve percentage increase calculations using raw byte values for accuracy.
Reviewer's GuideThe PR refactors CodeSizeAnalyzer to capture raw byte counts alongside formatted sizes, updates calculateBundleSize to return both formats, switches percentage computations to rely on raw bytes with zero-division safeguards, and adds tests for unit variance and empty bundles. File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey there - I've reviewed your changes and they look great!
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location> `src/utils/index.ts:267` </location>
<code_context>
private _log;
private originalSize: SizeResult;
private obfuscatedSize: SizeResult;
+ private originalBytes: { total: number; gzip: number };
+ private obfuscatedBytes: { total: number; gzip: number };
private startTime: number;
private endTime: number;
</code_context>
<issue_to_address>
Consider using a type alias for byte size objects.
Defining a type alias for the shared structure will make future updates easier and the code more readable.
Suggested implementation:
```typescript
type ByteSize = { total: number; gzip: number };
private _log;
private originalSize: SizeResult;
private obfuscatedSize: SizeResult;
private originalBytes: ByteSize;
private obfuscatedBytes: ByteSize;
private startTime: number;
private endTime: number;
```
```typescript
this._log = log;
this.originalSize = this.createEmptySizeResult();
this.obfuscatedSize = this.createEmptySizeResult();
this.originalBytes = { total: 0, gzip: 0 };
this.obfuscatedBytes = { total: 0, gzip: 0 };
this.startTime = 0;
this.endTime = 0;
}
```
</issue_to_address>
### Comment 2
<location> `src/__tests__/plugin.spec.ts:301` </location>
<code_context>
expect(lastCallArgs[1]).toContain('MB');
});
+
+ it('should calculate percentage correctly when units differ', () => {
+ const originalBundle: BundleList = [
+ ['test.js', { code: 'a'.repeat(500 * 1024) } as any]
+ ];
+
+ const obfuscatedBundle: BundleList = [
+ ['test.js', { code: 'a'.repeat(2 * 1024 * 1024) } as any]
+ ];
+
+ analyzer.start(originalBundle);
+ analyzer.end(obfuscatedBundle);
+
+ const lastCallArgs = logSpy.mock.lastCall;
+ expect(lastCallArgs[0]).toBe('\x1b[32m%s\x1b[0m');
+ const result = lastCallArgs[1];
+
+ expect(result).toMatch(/\d+(\.\d+)?KB.*→.*\d+(\.\d+)?MB/);
+ expect(result).toMatch(/309\.\d+%/);
+ });
+
</code_context>
<issue_to_address>
Consider adding assertions for gzip percentage increase and output format.
Adding assertions for gzip percentage and its output format will help verify that both raw and gzip metrics are correctly calculated and displayed, aligning with the updated implementation.
</issue_to_address>
<suggested_fix>
<<<<<<< SEARCH
expect(result).toMatch(/\d+(\.\d+)?KB.*→.*\d+(\.\d+)?MB/);
expect(result).toMatch(/309\.\d+%/);
});
=======
expect(result).toMatch(/\d+(\.\d+)?KB.*→.*\d+(\.\d+)?MB/);
expect(result).toMatch(/309\.\d+%/);
// Assert gzip output format (e.g., KB → MB)
expect(result).toMatch(/gzip:\s*\d+(\.\d+)?KB.*→.*\d+(\.\d+)?MB/);
// Assert gzip percentage increase (should be similar to raw, but calculated separately)
// The actual percentage may differ depending on gzip implementation, so match a percentage pattern
expect(result).toMatch(/gzip:.*\(\+\d+(\.\d+)?%\)/);
});
>>>>>>> REPLACE
</suggested_fix>
### Comment 3
<location> `src/__tests__/plugin.spec.ts:310` </location>
<code_context>
+ ['test.js', { code: 'a'.repeat(2 * 1024 * 1024) } as any]
+ ];
+
+ analyzer.start(originalBundle);
+ analyzer.end(obfuscatedBundle);
+
+ const lastCallArgs = logSpy.mock.lastCall;
</code_context>
<issue_to_address>
Missing test for negative percentage increase (obfuscated smaller than original).
Please add a test where the obfuscated bundle is smaller than the original to ensure negative percentage increases are handled and displayed correctly.
</issue_to_address>
### Comment 4
<location> `src/utils/index.ts:304` </location>
<code_context>
}
- private calculateBundleSize(bundleList: BundleList): { original: FormatSizeResult; gzip: FormatSizeResult } {
+ private calculateBundleSize(bundleList: BundleList): {
+ size: { original: FormatSizeResult; gzip: FormatSizeResult };
+ bytes: { total: number; gzip: number };
</code_context>
<issue_to_address>
Consider simplifying the code by returning and storing only raw byte sizes, and formatting sizes on-the-fly in the logging method.
```suggestion
// 1. Simplify calculateBundleSize to return raw bytes only
private calculateBundleSize(bundleList: BundleList): { total: number; gzip: number } {
const { totalSize, gzipSize } = bundleList.reduce(
(acc, [, bundleItem]) => {
if (bundleItem.code) {
const code = bundleItem.code;
acc.totalSize += Buffer.byteLength(code, 'utf-8');
acc.gzipSize += gzipSync(code, { level: 9 }).byteLength;
}
return acc;
},
{ totalSize: 0, gzipSize: 0 },
);
return { total: totalSize, gzip: gzipSize };
}
// 2. Drop originalSize/obfuscatedSize fields and only keep raw bytes
private originalBytes = { total: 0, gzip: 0 };
private obfuscatedBytes = { total: 0, gzip: 0 };
start(originalBundleList: BundleList): void {
this.startTime = performance.now();
this.originalBytes = this.calculateBundleSize(originalBundleList);
}
end(obfuscatedBundleList: BundleList): void {
this.obfuscatedBytes = this.calculateBundleSize(obfuscatedBundleList);
this.endTime = performance.now();
this.logResult();
}
// 3. Format sizes on-the-fly in analyze(), removing nested destructuring and guard logic
private analyze(): string {
const { total: oT, gzip: oG } = this.originalBytes;
const { total: fT, gzip: fG } = this.obfuscatedBytes;
const origSize = formatSize(oT);
const origGzip = formatSize(oG);
const newSize = formatSize(fT);
const newGzip = formatSize(fG);
const pct = oT > 0 ? (((fT - oT) / oT) * 100).toFixed(2) : '0.00';
const gzipPct = oG > 0 ? (((fG - oG) / oG) * 100).toFixed(2) : '0.00';
const timeTaken = formatTime(this.endTime - this.startTime);
return `✓ obfuscated in ${timeTaken} | `
+ `📦 ${origSize.value}${origSize.unit} (gzip: ${origGzip.value}${origGzip.unit}) → `
+ `🔒 ${newSize.value}${newSize.unit} (gzip: ${newGzip.value}${newGzip.unit}) | `
+ `📈 ${pct}% (gzip: ${gzipPct}%)`;
}
```
This keeps all functionality but removes the extra `size` object, nested destructuring, and boilerplate guard logic, by formatting sizes right before logging.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
+267
to
+268
| private originalBytes: { total: number; gzip: number }; | ||
| private obfuscatedBytes: { total: number; gzip: number }; |
There was a problem hiding this comment.
suggestion: Consider using a type alias for byte size objects.
Defining a type alias for the shared structure will make future updates easier and the code more readable.
Suggested implementation:
type ByteSize = { total: number; gzip: number };
private _log;
private originalSize: SizeResult;
private obfuscatedSize: SizeResult;
private originalBytes: ByteSize;
private obfuscatedBytes: ByteSize;
private startTime: number;
private endTime: number; this._log = log;
this.originalSize = this.createEmptySizeResult();
this.obfuscatedSize = this.createEmptySizeResult();
this.originalBytes = { total: 0, gzip: 0 };
this.obfuscatedBytes = { total: 0, gzip: 0 };
this.startTime = 0;
this.endTime = 0;
}
Comment on lines
+317
to
+319
| expect(result).toMatch(/\d+(\.\d+)?KB.*→.*\d+(\.\d+)?MB/); | ||
| expect(result).toMatch(/309\.\d+%/); | ||
| }); |
There was a problem hiding this comment.
suggestion (testing): Consider adding assertions for gzip percentage increase and output format.
Adding assertions for gzip percentage and its output format will help verify that both raw and gzip metrics are correctly calculated and displayed, aligning with the updated implementation.
Suggested change
| expect(result).toMatch(/\d+(\.\d+)?KB.*→.*\d+(\.\d+)?MB/); | |
| expect(result).toMatch(/309\.\d+%/); | |
| }); | |
| expect(result).toMatch(/\d+(\.\d+)?KB.*→.*\d+(\.\d+)?MB/); | |
| expect(result).toMatch(/309\.\d+%/); | |
| // Assert gzip output format (e.g., KB → MB) | |
| expect(result).toMatch(/gzip:\s*\d+(\.\d+)?KB.*→.*\d+(\.\d+)?MB/); | |
| // Assert gzip percentage increase (should be similar to raw, but calculated separately) | |
| // The actual percentage may differ depending on gzip implementation, so match a percentage pattern | |
| expect(result).toMatch(/gzip:.*\(\+\d+(\.\d+)?%\)/); | |
| }); |
Comment on lines
+310
to
+311
| analyzer.start(originalBundle); | ||
| analyzer.end(obfuscatedBundle); |
There was a problem hiding this comment.
suggestion (testing): Missing test for negative percentage increase (obfuscated smaller than original).
Please add a test where the obfuscated bundle is smaller than the original to ensure negative percentage increases are handled and displayed correctly.
| this.endTime = performance.now(); | ||
| this.logResult(); | ||
| } | ||
|
|
||
| private calculateBundleSize(bundleList: BundleList): { original: FormatSizeResult; gzip: FormatSizeResult } { | ||
| private calculateBundleSize(bundleList: BundleList): { |
There was a problem hiding this comment.
issue (complexity): Consider simplifying the code by returning and storing only raw byte sizes, and formatting sizes on-the-fly in the logging method.
Suggested change
| private calculateBundleSize(bundleList: BundleList): { | |
| // 1. Simplify calculateBundleSize to return raw bytes only | |
| private calculateBundleSize(bundleList: BundleList): { total: number; gzip: number } { | |
| const { totalSize, gzipSize } = bundleList.reduce( | |
| (acc, [, bundleItem]) => { | |
| if (bundleItem.code) { | |
| const code = bundleItem.code; | |
| acc.totalSize += Buffer.byteLength(code, 'utf-8'); | |
| acc.gzipSize += gzipSync(code, { level: 9 }).byteLength; | |
| } | |
| return acc; | |
| }, | |
| { totalSize: 0, gzipSize: 0 }, | |
| ); | |
| return { total: totalSize, gzip: gzipSize }; | |
| } | |
| // 2. Drop originalSize/obfuscatedSize fields and only keep raw bytes | |
| private originalBytes = { total: 0, gzip: 0 }; | |
| private obfuscatedBytes = { total: 0, gzip: 0 }; | |
| start(originalBundleList: BundleList): void { | |
| this.startTime = performance.now(); | |
| this.originalBytes = this.calculateBundleSize(originalBundleList); | |
| } | |
| end(obfuscatedBundleList: BundleList): void { | |
| this.obfuscatedBytes = this.calculateBundleSize(obfuscatedBundleList); | |
| this.endTime = performance.now(); | |
| this.logResult(); | |
| } | |
| // 3. Format sizes on-the-fly in analyze(), removing nested destructuring and guard logic | |
| private analyze(): string { | |
| const { total: oT, gzip: oG } = this.originalBytes; | |
| const { total: fT, gzip: fG } = this.obfuscatedBytes; | |
| const origSize = formatSize(oT); | |
| const origGzip = formatSize(oG); | |
| const newSize = formatSize(fT); | |
| const newGzip = formatSize(fG); | |
| const pct = oT > 0 ? (((fT - oT) / oT) * 100).toFixed(2) : '0.00'; | |
| const gzipPct = oG > 0 ? (((fG - oG) / oG) * 100).toFixed(2) : '0.00'; | |
| const timeTaken = formatTime(this.endTime - this.startTime); | |
| return `✓ obfuscated in ${timeTaken} | ` | |
| + `📦 ${origSize.value}${origSize.unit} (gzip: ${origGzip.value}${origGzip.unit}) → ` | |
| + `🔒 ${newSize.value}${newSize.unit} (gzip: ${newGzip.value}${newGzip.unit}) | ` | |
| + `📈 ${pct}% (gzip: ${gzipPct}%)`; | |
| } |
This keeps all functionality but removes the extra size object, nested destructuring, and boilerplate guard logic, by formatting sizes right before logging.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
originalBytesandobfuscatedBytesto store raw byte values.calculateBundleSizeto return both formatted sizes and raw byte data.Summary by Sourcery
Track raw total and gzip byte sizes in the code size analyzer, update bundle size calculations to return formatted and raw data, improve percentage increase accuracy, and add tests for unit variations and zero-size scenarios
New Features:
Bug Fixes:
Enhancements:
Tests: