perf(kit,nuxt): reduce unnecessary iteration in nuxt code #33212
Conversation
|
|
@nuxt/kit
nuxt
@nuxt/rspack-builder
@nuxt/schema
@nuxt/vite-builder
@nuxt/webpack-builder
commit: |
WalkthroughThe PR replaces many functional array operations with explicit imperative loops across numerous packages (kit, nuxt app/composables, components, pages, head, webpack, Nitro runtime and core). It changes addModuleTranspiles to accept a Nuxt instance and reworks module transpile detection/normalisation and importIncludes construction. Nitro now collects layer public asset dirs and builds exclude paths imperatively. renderer.normalizeChunks now drops whitespace‑only entries. Runtime config key enumeration switched to a for‑in loop (prototype keys may be included). A test snapshot updated server bundle size from "549k" to "550k". Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes
Pre-merge checks (3 passed)✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
packages/webpack/src/plugins/vue/client.ts (2)
94-103: Also filter out invalid indices for namedChunkGroup assets.
fileToIndex(asset.name)can return-1. Add the same guard here to keepmodules[...]clean.- for (const asset of chunkGroup.assets!) { - filesSet.add(fileToIndex(asset.name)) - } + for (const asset of chunkGroup.assets!) { + const idx = fileToIndex(asset.name) + if (idx !== -1) { + filesSet.add(idx) + } + }
105-106: De-duplicate and exclude-1when incorporating asset module indices.Appending
m.assets.map(fileToIndex)may add-1and duplicates. Use the set for dedupe and guard invalids.- const files = Array.from(filesSet) - webpackManifest.modules[hash(id)] = files + const files = Array.from(filesSet) + webpackManifest.modules[hash(id)] = files @@ - if (m.assets?.length && m.chunks?.includes(cid)) { - files.push(...m.assets.map(fileToIndex)) - } + if (m.assets?.length && m.chunks?.includes(cid)) { + for (const a of m.assets) { + const idx = fileToIndex(a) + if (idx !== -1 && !filesSet.has(idx)) { + filesSet.add(idx) + files.push(idx) + } + } + }Also applies to: 117-124
packages/nuxt/src/app/composables/preload.ts (1)
71-73: Protect against accidentalsplice(-1)when removing settled promises.If
promiseisn’t found,splice(-1)removes the last item.- .finally(() => promises.splice(promises.indexOf(promise))) + .finally(() => { + const i = promises.indexOf(promise) + if (i !== -1) { promises.splice(i, 1) } + })
🧹 Nitpick comments (7)
packages/nuxt/src/core/runtime/nitro/utils/renderer/build-files.ts (1)
121-130: Avoid non-null assertion onentry.src.Guard
entry.srcto prevent pushingundefinedif upstream typing changes.- for (const entry of Object.values(r)) { + for (const entry of Object.values(r)) { // @ts-expect-error internal key set by CSS inlining configuration - if (entry._globalCSS) { - entryIds.push(entry.src!) + if (entry._globalCSS && entry.src) { + entryIds.push(entry.src) } }packages/nuxt/src/app/composables/router.ts (1)
149-156: Join features without spaces to maximise browser compatibility.Some UAs are picky about the whitespace in
window.openfeature strings.join(',')is safer.- open(toPath, target, features.join(', ')) + open(toPath, target, features.join(','))packages/nuxt/src/app/nuxt.ts (1)
583-587: Avoid enumerating prototype keys from runtime config.
for...incan include inherited keys. Use own keys to keep the warning accurate.- const keys: string[] = [] - for (const key in runtimeConfig) { - keys.push(`\`${key}\``) - } + const keys = Object.keys(runtimeConfig).map(key => `\`${key}\``)packages/nuxt/src/pages/runtime/page.ts (1)
236-244: Widen parameter type to reflect actual call-sites (booleans/undefined included)
_mergeTransitionPropsreceivesboolean | TransitionProps | undefinedvalues (see Line 153). Update the signature to avoid TS friction and better document intent.-function _mergeTransitionProps (routeProps: TransitionProps[]): TransitionProps { +function _mergeTransitionProps (routeProps: Array<TransitionProps | false | undefined>): TransitionProps { const _props: TransitionProps[] = [] for (const prop of routeProps) { if (!prop) { continue } _props.push({ ...prop, onAfterLeave: prop.onAfterLeave ? toArray(prop.onAfterLeave) : undefined, }) } return defu(..._props as [TransitionProps, TransitionProps]) }packages/nuxt/src/components/plugins/islands-transform.ts (1)
170-176: Tiny clean-up: redundant truthiness check onname
Object.entriesguaranteesnameis truthy. Simplify the condition.- for (const [name, value] of Object.entries(bindings)) { - if (name && (name !== '_bind' && name !== 'v-for')) { + for (const [name, value] of Object.entries(bindings)) { + if (name !== '_bind' && name !== 'v-for') { contentParts.push(isBinding(name) ? `[\`${name.slice(1)}\`]: ${value}` : `[\`${name}\`]: \`${value}\``) } }packages/nuxt/src/core/nuxt.ts (2)
417-421: Normalise layer paths before checking/pushing (cross‑platform nit).Normalising once avoids separator edge cases on Windows and reduces false positives by testing for
node_modules/explicitly.- for (const layer of layerDirs) { - if (layer.root.includes('node_modules')) { - nuxt.options.build.transpile.push(layer.root.replace(/\/$/, '')) - } - } + for (const layer of layerDirs) { + const root = normalize(layer.root) + if (root.includes('node_modules/')) { + nuxt.options.build.transpile.push(root.replace(/\/$/, '')) + } + }
793-798: Normaliselayer.cwdbefore building the include regex.This prevents separator mismatches on Windows and keeps the regex precise. The pattern is escaped, so ReDoS risk is minimal; this change is purely for path normalisation.
- for (const layer of options._layers) { - if (layer.cwd && layer.cwd.includes('node_modules')) { - importIncludes.push(new RegExp(`(^|\\/)${escapeRE(layer.cwd.split('node_modules/').pop()!)}(\\/|$)(?!node_modules\\/)`)) - } - } + for (const layer of options._layers) { + const cwd = layer.cwd && normalize(layer.cwd) + if (cwd && cwd.includes('node_modules/')) { + importIncludes.push(new RegExp(`(^|\\/)${escapeRE(cwd.split('node_modules/').pop()!)}(\\/|$)(?!node_modules\\/)`)) + } + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (16)
packages/kit/src/loader/nuxt.ts(1 hunks)packages/nuxt/src/app/composables/preload.ts(1 hunks)packages/nuxt/src/app/composables/router.ts(1 hunks)packages/nuxt/src/app/nuxt.ts(1 hunks)packages/nuxt/src/components/plugins/islands-transform.ts(1 hunks)packages/nuxt/src/components/templates.ts(2 hunks)packages/nuxt/src/core/modules.ts(1 hunks)packages/nuxt/src/core/nitro.ts(4 hunks)packages/nuxt/src/core/nuxt.ts(3 hunks)packages/nuxt/src/core/runtime/nitro/handlers/renderer.ts(1 hunks)packages/nuxt/src/core/runtime/nitro/utils/renderer/build-files.ts(1 hunks)packages/nuxt/src/head/runtime/components.ts(2 hunks)packages/nuxt/src/pages/module.ts(3 hunks)packages/nuxt/src/pages/runtime/page.ts(1 hunks)packages/webpack/src/plugins/vue/client.ts(1 hunks)packages/webpack/src/plugins/vue/server.ts(2 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Follow standard TypeScript conventions and best practices
Files:
packages/nuxt/src/core/runtime/nitro/handlers/renderer.tspackages/nuxt/src/core/runtime/nitro/utils/renderer/build-files.tspackages/webpack/src/plugins/vue/client.tspackages/nuxt/src/app/composables/preload.tspackages/kit/src/loader/nuxt.tspackages/nuxt/src/components/templates.tspackages/nuxt/src/pages/runtime/page.tspackages/nuxt/src/pages/module.tspackages/nuxt/src/app/composables/router.tspackages/nuxt/src/head/runtime/components.tspackages/nuxt/src/app/nuxt.tspackages/nuxt/src/core/nitro.tspackages/webpack/src/plugins/vue/server.tspackages/nuxt/src/core/nuxt.tspackages/nuxt/src/core/modules.tspackages/nuxt/src/components/plugins/islands-transform.ts
🧠 Learnings (3)
📚 Learning: 2024-12-12T12:36:34.871Z
Learnt from: huang-julien
PR: nuxt/nuxt#29366
File: packages/nuxt/src/app/components/nuxt-root.vue:16-19
Timestamp: 2024-12-12T12:36:34.871Z
Learning: In `packages/nuxt/src/app/components/nuxt-root.vue`, when optimizing bundle size by conditionally importing components based on route metadata, prefer using inline conditional imports like:
```js
const IsolatedPage = route?.meta?.isolate ? defineAsyncComponent(() => import('#build/isolated-page.mjs')) : null
```
instead of wrapping the import in a computed property or importing the component unconditionally.
Applied to files:
packages/nuxt/src/app/composables/preload.tspackages/kit/src/loader/nuxt.tspackages/nuxt/src/pages/module.tspackages/nuxt/src/head/runtime/components.ts
📚 Learning: 2024-11-05T15:22:54.759Z
Learnt from: GalacticHypernova
PR: nuxt/nuxt#26468
File: packages/nuxt/src/components/plugins/loader.ts:24-24
Timestamp: 2024-11-05T15:22:54.759Z
Learning: In `packages/nuxt/src/components/plugins/loader.ts`, the references to `resolve` and `distDir` are legacy code from before Nuxt used the new unplugin VFS and will be removed.
Applied to files:
packages/kit/src/loader/nuxt.tspackages/webpack/src/plugins/vue/server.tspackages/nuxt/src/core/nuxt.ts
📚 Learning: 2024-11-05T20:04:39.622Z
Learnt from: Tofandel
PR: nuxt/nuxt#26468
File: packages/nuxt/src/components/runtime/client-delayed-component.ts:62-62
Timestamp: 2024-11-05T20:04:39.622Z
Learning: In Vue components, when typing the `hydrate` prop for hydration strategies like `hydrateOnInteraction`, use `type: null as PropType<Arguments<typeof hydrateOnInteraction>[0]>` to get the type directly from Vue's hydration methods. Also, remember that `HTMLElementEventMap` is not a type but an object; use `keyof HTMLElementEventMap` when defining prop types for events.
Applied to files:
packages/nuxt/src/head/runtime/components.ts
🧬 Code graph analysis (4)
packages/kit/src/loader/nuxt.ts (1)
packages/kit/src/internal/esm.ts (1)
directoryToURL(13-15)
packages/nuxt/src/pages/runtime/page.ts (1)
packages/nuxt/src/pages/runtime/utils.ts (1)
toArray(29-31)
packages/nuxt/src/core/nuxt.ts (1)
packages/nuxt/src/core/modules.ts (1)
addModuleTranspiles(5-30)
packages/nuxt/src/core/modules.ts (1)
packages/vite/src/utils/transpile.ts (1)
transpile(11-28)
🪛 ast-grep (0.38.6)
packages/nuxt/src/head/runtime/components.ts
[warning] 152-152: Direct HTML content assignment detected. Modifying innerHTML, outerHTML, or using document.write with unsanitized content can lead to XSS vulnerabilities. Use secure alternatives like textContent or sanitize HTML with libraries like DOMPurify.
Context: noscript.innerHTML = textContent.join('')
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://www.dhairyashah.dev/posts/why-innerhtml-is-a-bad-idea-and-how-to-avoid-it/
- https://cwe.mitre.org/data/definitions/79.html
(unsafe-html-content-assignment)
[warning] 152-152: Direct modification of innerHTML or outerHTML properties detected. Modifying these properties with unsanitized user input can lead to XSS vulnerabilities. Use safe alternatives or sanitize content first.
Context: noscript.innerHTML = textContent.join('')
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://owasp.org/www-community/xss-filter-evasion-cheatsheet
- https://cwe.mitre.org/data/definitions/79.html
(dom-content-modification)
packages/nuxt/src/core/nuxt.ts
[warning] 795-795: Regular expression constructed from variable input detected. This can lead to Regular Expression Denial of Service (ReDoS) attacks if the variable contains malicious patterns. Use libraries like 'recheck' to validate regex safety or use static patterns.
Context: new RegExp((^|\\/)${escapeRE(layer.cwd.split('node_modules/').pop()!)}(\\/|$)(?!node_modules\\/))
Note: [CWE-1333] Inefficient Regular Expression Complexity [REFERENCES]
- https://owasp.org/www-community/attacks/Regular_expression_Denial_of_Service_-_ReDoS
- https://cwe.mitre.org/data/definitions/1333.html
(regexp-from-variable)
⏰ 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). (20)
- GitHub Check: test-fixtures (windows-latest, built, vite, default, manifest-on, json, lts/-1)
- GitHub Check: test-fixtures (windows-latest, built, vite, async, manifest-off, json, lts/-1)
- GitHub Check: test-fixtures (windows-latest, dev, vite, default, manifest-off, json, lts/-1)
- GitHub Check: test-fixtures (ubuntu-latest, built, webpack, default, manifest-on, json, lts/-1)
- GitHub Check: test-fixtures (ubuntu-latest, built, webpack, async, manifest-on, json, lts/-1)
- GitHub Check: test-fixtures (ubuntu-latest, built, vite, default, manifest-on, json, lts/-1)
- GitHub Check: test-fixtures (ubuntu-latest, built, vite, default, manifest-off, json, lts/-1)
- GitHub Check: test-fixtures (ubuntu-latest, dev, vite, default, manifest-off, json, lts/-1)
- GitHub Check: test-fixtures (ubuntu-latest, dev, vite, async, manifest-off, json, lts/-1)
- GitHub Check: test-fixtures (ubuntu-latest, built, vite, async, manifest-on, js, lts/-1)
- GitHub Check: test-fixtures (ubuntu-latest, built, vite, async, manifest-off, json, lts/-1)
- GitHub Check: test-fixtures (ubuntu-latest, dev, vite, default, manifest-on, json, lts/-1)
- GitHub Check: test-fixtures (ubuntu-latest, dev, vite, async, manifest-on, json, lts/-1)
- GitHub Check: test-fixtures (ubuntu-latest, dev, vite, async, manifest-on, js, lts/-1)
- GitHub Check: release-pkg-pr-new
- GitHub Check: typecheck (ubuntu-latest, bundler)
- GitHub Check: test-benchmark
- GitHub Check: typecheck (windows-latest, bundler)
- GitHub Check: test-size
- GitHub Check: code
🔇 Additional comments (19)
packages/webpack/src/plugins/vue/client.ts (1)
86-92: Good change: avoid invalid (-1) indices for chunk.files.The explicit loop with an index check prevents leaking
-1into the manifest. Looks solid.packages/webpack/src/plugins/vue/server.ts (1)
58-73: LGTM: simpler iteration without behaviour change.The switch to
for...ofkeeps logic identical and is easier to follow.packages/nuxt/src/app/composables/preload.ts (1)
64-69: Good guard against non-callable defaults.Prevents runtime errors when a matched route’s
defaultisn’t a loader function.packages/nuxt/src/pages/module.ts (3)
231-233: LGTM: explicit loop for child deletion.Clearer than chaining and identical in effect.
267-270: LGTM: explicit iteration overpage.children.Keeps behaviour and improves readability.
561-576: Confirm desired precedence forhashModeresolution.Using
unshiftduring a forward index loop makes later files take precedence. Please confirm this matches layering/override semantics across_layers, especially with built-in defaults and user overrides.packages/nuxt/src/core/runtime/nitro/handlers/renderer.ts (1)
329-336: Behaviour change: whitespace-only chunks are now dropped — confirm no callers rely on empty stringsPreviously,
' 'became''and was retained; now it’s omitted. This can alter array lengths (e.g., head/body chunk arrays). If intentional, consider a brief comment or test to lock this in.packages/nuxt/src/head/runtime/components.ts (1)
152-154: InnerHTML assignment — confirm trusted content and document rationaleThis mirrors prior behaviour, but static analysis flags XSS risk. If slot content is always developer-authored (not user input), add a comment to clarify. Otherwise, consider sanitisation or restricting to plain text.
packages/nuxt/src/components/templates.ts (2)
68-74: LGTM: same semantics with fewer intermediate allocationsIteration reads clearly and avoids extra arrays.
115-127: LGTM: clear accumulation of component typesLogic for server-only/island typing preserved; explicit loop improves readability.
packages/nuxt/src/core/nitro.ts (5)
37-49: Cross‑platform check:dirs.root.match(NODE_MODULES_RE)on WindowsThese regexes expect POSIX separators. If
dirs.rootcan contain backslashes, matches will fail. Consider normalisingdirs.rootto POSIX before matching or adjusting patterns.- const paths = [ - dirs.root.match(NODE_MODULES_RE)?.[1]?.replace(/\/$/, ''), - dirs.root.match(PNPM_NODE_MODULES_RE)?.[1]?.replace(/\/$/, ''), - ] + const rootPosix = dirs.root.replace(/\\/g, '/') + const paths = [ + rootPosix.match(NODE_MODULES_RE)?.[1]?.replace(/\/$/, ''), + rootPosix.match(PNPM_NODE_MODULES_RE)?.[1]?.replace(/\/$/, ''), + ]
50-55: LGTM: precompute layer public asset dirsAvoids repeated directory scans later; straightforward and safe.
174-174: LGTM: TS include now derived from cachedlayerDirsKeeps logic local and avoids recomputing.
191-191: LGTM: reuselayerPublicAssetsDirsMatches the intent to collect once and consume later.
217-217: LGTM: inline app.config paths per layerConsistent with prior
layerDirsusage.packages/nuxt/src/core/modules.ts (1)
19-29: Function-based transpile entries are ignored for de-duplication — confirm acceptableIf users provide a function that resolves to a pattern matching
path, we may still appendpathhere. If that’s fine (idempotent at consumer sites), ignore. Otherwise, consider evaluating known trivial functions or documenting this behaviour.packages/nuxt/src/core/nuxt.ts (3)
714-721: Transpile entry normalisation looks good.Covers Windows by using
normalizeand retains non-string entries unchanged.
722-722: API usage aligned with new signature.Passing
nuxtintoaddModuleTranspilesmatches the updated export.
801-801: Using a precomputedimportIncludesis a clear improvement.Keeps transform scope explicit and avoids rebuilding patterns repeatedly.
| const textContent: VNodeNormalizedChildren[] = [] | ||
| if (slotVnodes) { | ||
| for (const vnode of slotVnodes) { | ||
| if (vnode.children) { | ||
| textContent.push(vnode.children) | ||
| } | ||
| } | ||
| } | ||
| if (textContent.length > 0) { | ||
| noscript.innerHTML = textContent.join('') | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Avoid joining arbitrary VNode children — normalise to strings to prevent commas/[object Object]
VNodeNormalizedChildren can be arrays/objects; join('') will coerce to unwanted strings (e.g., commas or [object Object]). Collect only string/number children (and flatten arrays) before joining.
- const textContent: VNodeNormalizedChildren[] = []
- if (slotVnodes) {
- for (const vnode of slotVnodes) {
- if (vnode.children) {
- textContent.push(vnode.children)
- }
- }
- }
- if (textContent.length > 0) {
- noscript.innerHTML = textContent.join('')
- }
+ const parts: string[] = []
+ if (slotVnodes) {
+ for (const vnode of slotVnodes) {
+ const c = vnode.children
+ if (typeof c === 'string' || typeof c === 'number') {
+ parts.push(String(c))
+ } else if (Array.isArray(c)) {
+ for (const cc of c) {
+ if (typeof cc === 'string' || typeof cc === 'number') {
+ parts.push(String(cc))
+ }
+ }
+ }
+ }
+ }
+ if (parts.length > 0) {
+ noscript.innerHTML = parts.join('')
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const textContent: VNodeNormalizedChildren[] = [] | |
| if (slotVnodes) { | |
| for (const vnode of slotVnodes) { | |
| if (vnode.children) { | |
| textContent.push(vnode.children) | |
| } | |
| } | |
| } | |
| if (textContent.length > 0) { | |
| noscript.innerHTML = textContent.join('') | |
| } | |
| const parts: string[] = [] | |
| if (slotVnodes) { | |
| for (const vnode of slotVnodes) { | |
| const c = vnode.children | |
| if (typeof c === 'string' || typeof c === 'number') { | |
| parts.push(String(c)) | |
| } else if (Array.isArray(c)) { | |
| for (const cc of c) { | |
| if (typeof cc === 'string' || typeof cc === 'number') { | |
| parts.push(String(cc)) | |
| } | |
| } | |
| } | |
| } | |
| } | |
| if (parts.length > 0) { | |
| noscript.innerHTML = parts.join('') | |
| } |
🧰 Tools
🪛 ast-grep (0.38.6)
[warning] 152-152: Direct HTML content assignment detected. Modifying innerHTML, outerHTML, or using document.write with unsanitized content can lead to XSS vulnerabilities. Use secure alternatives like textContent or sanitize HTML with libraries like DOMPurify.
Context: noscript.innerHTML = textContent.join('')
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://www.dhairyashah.dev/posts/why-innerhtml-is-a-bad-idea-and-how-to-avoid-it/
- https://cwe.mitre.org/data/definitions/79.html
(unsafe-html-content-assignment)
[warning] 152-152: Direct modification of innerHTML or outerHTML properties detected. Modifying these properties with unsanitized user input can lead to XSS vulnerabilities. Use safe alternatives or sanitize content first.
Context: noscript.innerHTML = textContent.join('')
Note: [CWE-79] Improper Neutralization of Input During Web Page Generation [REFERENCES]
- https://owasp.org/www-community/xss-filter-evasion-cheatsheet
- https://cwe.mitre.org/data/definitions/79.html
(dom-content-modification)
🤖 Prompt for AI Agents
In packages/nuxt/src/head/runtime/components.ts around lines 144 to 154, avoid
calling join('') on arbitrary VNodeNormalizedChildren (which can be
arrays/objects) — instead iterate and normalise children to plain strings:
flatten any arrays, extract only string or number values (convert numbers with
String()), and for VNode/text nodes use their text content; ignore non-text
objects. After building the cleaned string, assign it via noscript.textContent
(or explicitly escape) rather than setting innerHTML to prevent commas/[object
Object] and potential injection.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
test/bundle.test.ts (1)
154-165: Avoid reading file contents to measure sizeUse
stat.sizeinstead ofreadFile+Buffer.byteLengthto cut I/O and memory for large bundles.- if (!isSymlink) { - const bytes = Buffer.byteLength(await fsp.readFile(path)) - totalBytes += bytes - } + if (!isSymlink) { + const bytes = (await fsp.stat(path)).size + totalBytes += bytes + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
test/bundle.test.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Follow standard TypeScript conventions and best practices
Files:
test/bundle.test.ts
**/*.{test,spec}.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Write unit tests for core functionality using
vitest
Files:
test/bundle.test.ts
⏰ 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). (12)
- GitHub Check: test-fixtures (windows-latest, built, webpack, default, manifest-on, json, lts/-1)
- GitHub Check: test-fixtures (ubuntu-latest, built, vite, default, manifest-off, json, lts/-1)
- GitHub Check: test-fixtures (windows-latest, built, vite, default, manifest-on, json, lts/-1)
- GitHub Check: test-fixtures (windows-latest, dev, vite, default, manifest-on, json, lts/-1)
- GitHub Check: test-fixtures (windows-latest, dev, vite, default, manifest-off, json, lts/-1)
- GitHub Check: test-fixtures (windows-latest, built, webpack, async, manifest-on, json, lts/-1)
- GitHub Check: test-fixtures (ubuntu-latest, built, vite, async, manifest-on, json, lts/-1)
- GitHub Check: test-fixtures (windows-latest, built, vite, default, manifest-off, json, lts/-1)
- GitHub Check: test-fixtures (windows-latest, built, vite, async, manifest-off, json, lts/-1)
- GitHub Check: test-fixtures (windows-latest, built, rspack, async, manifest-on, json, lts/-1)
- GitHub Check: test-fixtures (ubuntu-latest, dev, vite, async, manifest-on, js, lts/-1)
- GitHub Check: test-benchmark
🔇 Additional comments (1)
test/bundle.test.ts (1)
98-98: Snapshot bump to 550k is fineTiny (+1k) shift is expected given code churn; keeping the snapshot in sync is appropriate here.
CodSpeed Performance ReportMerging #33212 will not alter performanceComparing Summary
|
🔗 Linked issue
📚 Description
building on work by @GalacticHypernova, this replaces a number of chained array methods like
.map().filter().map()etc.many of these functions are called over and over again