Skip to content

fix info script so timing script doesn't fail if undefined#7

Merged
nucliweb merged 2 commits into
mainfrom
fix-script-info-timings
Jul 18, 2022
Merged

fix info script so timing script doesn't fail if undefined#7
nucliweb merged 2 commits into
mainfrom
fix-script-info-timings

Conversation

@jhadev

@jhadev jhadev commented Jul 16, 2022

Copy link
Copy Markdown
Contributor

changed this code to avoid empty arrays without objects with 'name' property

const scripts = {
    firstParty: [{ name: "no data" }],
    thirdParty: [{ name: "no data" }],
  };

  if (first.length) {
    scripts.firstParty = first;
  }

  if (third.length) {
    scripts.thirdParty = third;
  }

return scripts

Tested by running on apple.com which has no third party scripts. Note timing script does not fail.

pic-Google Chrome-Apple-07-152022 2

Previous snippet caused this error on apple.com

pic-Google Chrome-Buy 14-inch MacBook Pro - Apple-07-162022

@jhadev jhadev linked an issue Jul 16, 2022 that may be closed by this pull request
@jhadev jhadev requested a review from nucliweb July 16, 2022 02:50
@jhadev jhadev added the bug Something isn't working label Jul 16, 2022
@jhadev

jhadev commented Jul 18, 2022

Copy link
Copy Markdown
Contributor Author

Cleaned up .gitignore, only beta and .DS_Store are in there now.

@nucliweb nucliweb left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👏

Comment thread .gitignore Outdated
other.js
.DS_Store No newline at end of file
.DS_Store
long-tasks.js No newline at end of file

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What do you think to add a "beta" o "canary" folder to add the WIP scripts instead of adding a new file to gitignore every time?

@nucliweb nucliweb merged commit a1cbe10 into main Jul 18, 2022
@nucliweb nucliweb deleted the fix-script-info-timings branch July 18, 2022 21:01
nucliweb added a commit that referenced this pull request Feb 12, 2026
Validate-Preload-Async-Defer-Scripts:
- Add missing fetchpriority="low" to <link rel=preload> tag in Solution 1
  This was a correctness bug - preload without fetchpriority="low" still
  escalates network priority even if script tag has fetchpriority="low"

Prefetch-Resource-Validation (comprehensive improvements):

Performance optimizations:
- Pre-normalize all URLs once into a Map (eliminates O(N×M) URL parsing)
- Use pre-built URL map for all matching operations
- Remove redundant URL parsing in nested loops
- Performance improvement: ~6000 URL objects → ~200 for typical cases

New validations:
- Detect invalid 'as' attribute values (typos like as="scriptt")
- Detect duplicate prefetch hints (same URL prefetched multiple times)
- Warn about large scripts (>1MB threshold, was missing)
- Validate 'as' attribute against list of valid values

Improved logic:
- Better cache detection: distinguish "cached" vs "unknown (CORS)"
  Handles case where CORS blocks timing info (both sizes === 0)
- Fixed inappropriateTypes to check scripts and provide detailed reasons
- Optimized isCurrentPageResource check to use pre-built map
- Added seenUrls Set to track duplicates

Code quality:
- Added validAsValues Set with all valid 'as' attribute values
- Added THRESHOLDS.largeScriptSize (1MB) for script-specific checks
- Better error messages with specific size/reason for inappropriateness
- Comments explaining CORS edge cases and validation logic

Documentation updates:
- Added "Invalid as", "Duplicate prefetch" to issue categories table
- Updated "Inappropriate type" description to mention scripts
- Cache status can now show "unknown (CORS)" in output

Addresses issues from comprehensive code review:
- URL matching performance (#8, #9 from analysis)
- Invalid 'as' validation (#7)
- Duplicate detection (#14)
- Large script detection (#4)
- Cache detection for CORS (#3)
- Current page detection optimization (#6)
nucliweb added a commit that referenced this pull request Feb 25, 2026
Validate-Preload-Async-Defer-Scripts:
- Add missing fetchpriority="low" to <link rel=preload> tag in Solution 1
  This was a correctness bug - preload without fetchpriority="low" still
  escalates network priority even if script tag has fetchpriority="low"

Prefetch-Resource-Validation (comprehensive improvements):

Performance optimizations:
- Pre-normalize all URLs once into a Map (eliminates O(N×M) URL parsing)
- Use pre-built URL map for all matching operations
- Remove redundant URL parsing in nested loops
- Performance improvement: ~6000 URL objects → ~200 for typical cases

New validations:
- Detect invalid 'as' attribute values (typos like as="scriptt")
- Detect duplicate prefetch hints (same URL prefetched multiple times)
- Warn about large scripts (>1MB threshold, was missing)
- Validate 'as' attribute against list of valid values

Improved logic:
- Better cache detection: distinguish "cached" vs "unknown (CORS)"
  Handles case where CORS blocks timing info (both sizes === 0)
- Fixed inappropriateTypes to check scripts and provide detailed reasons
- Optimized isCurrentPageResource check to use pre-built map
- Added seenUrls Set to track duplicates

Code quality:
- Added validAsValues Set with all valid 'as' attribute values
- Added THRESHOLDS.largeScriptSize (1MB) for script-specific checks
- Better error messages with specific size/reason for inappropriateness
- Comments explaining CORS edge cases and validation logic

Documentation updates:
- Added "Invalid as", "Duplicate prefetch" to issue categories table
- Updated "Inappropriate type" description to mention scripts
- Cache status can now show "unknown (CORS)" in output

Addresses issues from comprehensive code review:
- URL matching performance (#8, #9 from analysis)
- Invalid 'as' validation (#7)
- Duplicate detection (#14)
- Large script detection (#4)
- Cache detection for CORS (#3)
- Current page detection optimization (#6)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3rd party script timing failing

2 participants