The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Actions	Updated (UTC)
webperf-snippets	Ready	Preview, Comment	Feb 11, 2026 3:03pm
webperf-snippets-u6am	Ready	Preview, Comment	Feb 11, 2026 3:03pm

PR Review: Add snippet to validate preload on async/defer scripts

Great addition! This addresses a real-world performance anti-pattern that's commonly misunderstood. The documentation is thorough and well-organized. Here's my detailed feedback:

Strengths

• Excellent educational value: The tables comparing script loading strategies and priority escalation are very clear and actionable.
• Well-structured snippet: Good use of IIFE, clear separation of concerns, meaningful variable names.
• Balanced detection: The snippet distinguishes between true anti-patterns, valid use cases, and dynamic imports, avoiding false positives.
• Comprehensive output: console.group/table usage makes the DevTools output well-organized.
• Good documentation: Real-world examples, further reading links, and the V8 diagram all add value.

Code Issues

Variable shadowing (bug risk):

Inside the forEach callback, a variable named location is declared:

const inHead = matchingScript.closest('head') \!== null;
const location = inHead ? 'head' : 'body'; // shadows window.location

This shadows the global window.location object, which is used earlier in the outer scope:

const preloadUrl = new URL(preloadHref, location.origin).href; // uses window.location.origin

While it does not cause a runtime bug in the current code (the outer preloadUrl is already resolved before the inner location is declared), it is a maintenance hazard. Future edits could accidentally rely on location inside the callback expecting it to be the global. Rename it to scriptLocation or placement.

Fragile URL matching for dynamic scripts:

const shortUrl = preloadHref.split('/').pop()?.split('?')[0] || preloadHref;
// ...
return entry.name === preloadUrl || entry.name.includes(shortUrl);

The includes(shortUrl) fallback could produce false positives for common filenames like main.js, index.js, or app.js. A script named main.js could match unrelated Performance API entries. Consider comparing URL pathnames more precisely, or at minimum documenting this limitation.

Module scripts handling:

The snippet flags type="module" scripts as part of the anti-pattern, but the output can combine async + type='module' in the attributes string. Since ES modules already behave like defer by default, and rel="modulepreload" (not rel="preload" as="script") is the correct hint for them, consider adding a specific message for module scripts pointing to modulepreload as the correct alternative.

Documentation Suggestions

Contradictory framing in "When Is Preload + Script Actually Appropriate?":

The section introduces a valid edge case for preload + async, but this somewhat contradicts the anti-pattern framing in the Overview table. The section ends with "Warning: This is a rare edge case. Most scripts should NOT be preloaded." Consider being more definitive upfront, or clearly separating the edge case earlier in the section.

Unsourced performance impact claim:

"Can delay LCP by 100-500ms on slower connections"

This is presented as a fact without a citation. The actual impact is highly site-specific. Consider softening to "can measurably delay LCP" or linking to a specific measurement/study.

Missing: fetchpriority as a mitigation:

The snippet recommends removing preload as the fix, but does not mention fetchpriority="low" on the script tag as an alternative that can preserve the preload for discovery while reducing its bandwidth priority. Worth adding as an option in the "Fix" guidance or Best Practices section.

Minor Notes

• _meta.json entry is correct.
• copy prop on the code fence follows CLAUDE.md conventions.
• Consider mentioning rel="modulepreload" as the correct alternative when preloading ES modules, since developers encountering this snippet may need to know the proper API.

Summary

This is a high-quality, well-researched snippet that fills a real gap in the collection. The main actionable fixes before merging:

1. Rename the location variable to avoid shadowing window.location
2. Tighten the Performance API URL matching to reduce false positives on common filenames
3. Add a clarification or specific handling for type="module" scripts pointing to rel="modulepreload"

The documentation content is excellent and the snippet logic is sound. With the variable shadowing fix in particular, this is ready to merge.

PR Review: Add snippet to validate preload on async/defer scripts

Overall this is a well-conceived and technically accurate addition to the collection. The documentation is thorough and the snippet addresses a real, commonly misunderstood performance anti-pattern. Here is detailed feedback.

Strengths

• Technically accurate content. The explanation of priority escalation (Lowest/Low to Medium/High) is correct and well-sourced with links to Addy Osmani, DebugBear, MDN, and web.dev.
• Handles edge cases. The snippet correctly handles orphan preloads, dynamic/code-split scripts (via Performance API), ES modules (type=module), and the fetchpriority=low mitigation.
• Good console UX. Use of console.group, console.table, colored styling, and structured output is consistent with the diagnostic nature of these snippets.
• _meta.json updated correctly.

Issues and Suggestions

1. URL matching logic — potential false positives

In the dynamic script detection block, the endsWith comparison can produce false positives for common filenames (e.g. main.js, index.js, chunk.js):

return entryUrl.pathname.endsWith(preloadUrlObj.pathname);

If a preload points to /app/main.js and a completely unrelated /vendor/main.js was loaded, this would match. A safer approach:

return entry.name === preloadUrl ||
       (entryUrl.pathname === preloadUrlObj.pathname &&
        entryUrl.origin === preloadUrlObj.origin);

2. Module detection — implicit async module case

The module check fires only when isAsync and isDefer are both false. However, an async ES module (script type=module async) would be caught by the isAsync branch first, skipping the modulepreload recommendation. The behavior is acceptable, but the comment about ES modules without async/defer behaving like defer could mislead future maintainers — worth clarifying in a note.

3. fetchpriority check — missing on the link element

The mitigation check only inspects fetchpriority on the script element. The attribute can also be set on the link rel=preload element itself. A developer writing link rel=preload with fetchpriority=low on the link tag would still trigger an error, even though this is a valid mitigation. Consider also checking fetchpriority on the preload link element and OR-ing it into the hasMitigation check.

4. Summary count label is misleading

The summary line reads: Valid preloads (blocking scripts): N

But validPreloads also contains dynamic scripts and mitigated preloads, not just blocking scripts. The label should be generalized (e.g. Valid or acceptable preloads) or the count filtered to only blocking scripts.

5. Minor: external image URL

The v8.dev diagram uses an external URL with no fallback. If that URL ever becomes unavailable the page will show a broken image. The project already uses Cloudinary for media — hosting it there would be more resilient.

Summary

This is a high-quality, technically sound contribution. The main actionable items before merging are:

1. Fix the false-positive risk in the endsWith pathname matching (issue 1)
2. Also check fetchpriority on the link rel=preload element (issue 3)
3. Fix the misleading Valid preloads (blocking scripts) counter label (issue 4)

Issues 2 and 5 are minor and can be addressed at the author's discretion.