Fix: use minified size in sourcemap mode #164
Conversation
|
Hi @btd , could you take a look? |
|
Hey @btd, did you have a chance to look through the changes? It's not too complicated, and should greatly improve the analysis. |
| @@ -83,7 +83,6 @@ export interface PluginVisualizerOptions { | |||
|
|
|||
| /** | |||
| * If plugin should use sourcemap to calculate sizes of modules. By idea it will present more accurate results. | |||
| * `gzipSize` and `brotliSize` does not make much sense with this option. | |||
There was a problem hiding this comment.
Why you removed this? You also disabled this in code.
| const gzipSize = !!opts.gzipSize && !opts.sourcemap; | ||
| const brotliSize = !!opts.brotliSize && !opts.sourcemap; | ||
| const gzipSize = !!opts.gzipSize; | ||
| const brotliSize = !!opts.brotliSize; |
There was a problem hiding this comment.
I would print a warning also. It is not good UX to just silently disable options.
| code, | ||
| }: { | ||
| id: string; | ||
| renderedLength: number; | ||
| code: string | null; |
There was a problem hiding this comment.
Please return this back. It is to support old 2.x version of rollup
| @@ -214,9 +211,8 @@ export const visualizer = ( | |||
| const moduleRenderInfo = await Promise.all( | |||
| Object.values(modules) | |||
| .filter(({ id }) => filter(bundleId, id)) | |||
| .map(({ id, renderedLength }) => { | |||
| const code = bundle.modules[id]?.code; | |||
| return ModuleLengths({ id, renderedLength, code }); | |||
There was a problem hiding this comment.
The same as above. Code was added only in the middle of 2.x (i support it)
| @@ -225,7 +221,7 @@ export const visualizer = ( | |||
| const modules = await Promise.all( | |||
| Object.entries(bundle.modules) | |||
| .filter(([id]) => filter(bundleId, id)) | |||
| .map(([id, { renderedLength, code }]) => ModuleLengths({ id, renderedLength, code })) | |||
| .map(([id, { code }]) => ModuleLengths({ id, code: code || "" })) | |||
| modules[id] = modules[id] || { id, renderedLength: 0 }; | ||
| modules[id].renderedLength += 1; | ||
| modules[id] = modules[id] || { id, code: "" }; | ||
| modules[id].code += code[i]; |
There was a problem hiding this comment.
There is no reason to try to combine code there. gzip and brotli sizes disabled for it. To get length need to get code point and get its length as surrogate pair
| rendered: 12 | ||
| rendered: 13 | ||
| gzip: 33 | ||
| brotli: 17 |
Hi @btd! As discussed yesterday in #163, fixed sourcemap mode to correctly measure module size after minification. To address your concern about surrogate pairs, I collect actual
codein the final chunk per-module instead of incrementingrenderedLength.Since we now have the precise code available in sourcemap mode, I also went ahead and allowed
gzipSizeandbrotliSizein sourcemap mode. Compressed sizes are very close to ones in default mode.Added a test case with terser, updated snapshots for both changes. Un-minified sourcemap sizes are 1 byte larger than
bundle.code[id]because rollup trims trailing newline.I wonder if we could remove non-sourcemap option altogether? It doesn't seem to have any benefits, and reports incorrect sizes.
Fixes #163