Conversation
There was a problem hiding this comment.
Sorry @ArgoZhang, your pull request is larger than the review limit of 150000 diff characters
There was a problem hiding this comment.
Pull Request Overview
This PR bumps the version of BootstrapBlazor.Dom2Image from 10.0.0 to 10.0.1, replacing the minified snapdom library with its unminified source version.
Key Changes
- Version incremented from 10.0.0 to 10.0.1 in the project file
- Replaced minified library (snapdom.min.mjs) with unminified source (snapdom.mjs, 5257 lines)
- Updated import statement to reference the unminified version
Reviewed Changes
Copilot reviewed 3 out of 4 changed files in this pull request and generated 6 comments.
| File | Description |
|---|---|
| BootstrapBlazor.Dom2Image.csproj | Version bumped to 10.0.1 |
| wwwroot/dom2image.js | Import changed from minified to unminified snapdom library |
| wwwroot/lib/snapdom.mjs | New unminified SnapDOM v2.0.0 library added (5257 lines) |
| wwwroot/lib/snapdom.min.mjs | Minified version removed |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| @@ -1,4 +1,4 @@ | |||
| import { snapdom } from './lib/snapdom.min.mjs' | |||
| import { snapdom } from './lib/snapdom.mjs' | |||
There was a problem hiding this comment.
The import statement is now pointing to the unminified version snapdom.mjs instead of snapdom.min.mjs. This will significantly increase the bundle size and impact load performance. For production use, it's recommended to use the minified version unless debugging is specifically needed.
Consider either:
- Reverting to
./lib/snapdom.min.mjsfor production builds - Using conditional imports based on build configuration (debug vs release)
| import { snapdom } from './lib/snapdom.mjs' | |
| import { snapdom } from './lib/snapdom.min.mjs' |
| try { | ||
| const required = collectUsedFontVariants(root); | ||
| const usedCodepoints = collectUsedCodepoints(root); | ||
| const safari = typeof isSafari === "function" ? isSafari() : !!isSafari; |
There was a problem hiding this comment.
This guard always evaluates to true.
| const safari = typeof isSafari === "function" ? isSafari() : !!isSafari; | |
| const safari = isSafari(); |
| const cs = getComputedStyle(el); | ||
| if (cs.display === "list-item" && isLi(el)) { | ||
| const p = el.parentElement; | ||
| let idx = 1; |
There was a problem hiding this comment.
The initial value of idx is unused, since it is always overwritten.
| let idx = 1; | |
| let idx; |
| }; | ||
| const optW = coerceNum(state.options.width); | ||
| const optH = coerceNum(state.options.height); | ||
| let w = w0, h = h0; |
There was a problem hiding this comment.
The initial value of w is unused, since it is always overwritten.
| let w = w0, h = h0; | |
| let w, h; |
| }; | ||
| const optW = coerceNum(state.options.width); | ||
| const optH = coerceNum(state.options.height); | ||
| let w = w0, h = h0; |
There was a problem hiding this comment.
The initial value of h is unused, since it is always overwritten.
| let w = w0, h = h0; | |
| let w, h; |
| console.warn("inlineExternal defs or symbol failed:", e); | ||
| } | ||
| try { | ||
| clone = await deepClone(element, sessionCache, options, element); |
There was a problem hiding this comment.
Superfluous argument passed to function deepClone.
| clone = await deepClone(element, sessionCache, options, element); | |
| clone = await deepClone(element, sessionCache, options); |
Link issues
fixes #708
Summary By Copilot
Regression?
Risk
Verification
Packaging changes reviewed?
☑️ Self Check before Merge