Skip to content

Conversation

@Uzlopak
Copy link
Contributor

@Uzlopak Uzlopak commented Sep 6, 2025

This relates to...

Rationale

Changes

Features

Bug Fixes

Breaking Changes and Deprecations

Status

@Uzlopak Uzlopak requested a review from Copilot September 6, 2025 06:15

This comment was marked as outdated.

Uzlopak and others added 2 commits September 6, 2025 08:36
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@Uzlopak Uzlopak requested a review from Copilot September 6, 2025 06:37

This comment was marked as outdated.

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@Uzlopak Uzlopak requested a review from Copilot September 6, 2025 06:45

This comment was marked as outdated.

@Uzlopak Uzlopak requested a review from Copilot September 6, 2025 06:55

This comment was marked as outdated.

@Uzlopak Uzlopak requested a review from Copilot September 6, 2025 21:18
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR extracts infrastructure and encoding methods from fetch-specific modules into dedicated, reusable modules. The refactoring consolidates commonly-used utility functions related to text processing, encoding, and WHATWG specification implementations into centralized locations.

Key changes:

  • Creates new lib/web/infra module with WHATWG infra specification utilities
  • Creates new lib/encoding module for text encoding/decoding functions
  • Updates import statements across multiple modules to use the new centralized utilities

Reviewed Changes

Copilot reviewed 15 out of 15 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
lib/web/infra/index.js New module containing WHATWG infra spec utilities like collectASequenceOfCodePoints, forgivingBase64, and JSON serialization
lib/encoding/index.js New module containing UTF-8 decoding functionality
test/infra/collect-a-sequence-of-code-points.js New test file for infra utilities
lib/web/fetch/data-url.js Removes extracted functions and imports from new infra module
lib/web/fetch/util.js Removes extracted functions and imports from new modules
Multiple fetch modules Updates imports to use new centralized modules
package.json Adds new test script for infra tests
.github/workflows/nodejs.yml Adds infra test step to CI workflow

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

Copy link
Member

@metcoder95 metcoder95 left a comment

Choose a reason for hiding this comment

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

Shall we maybe namespace it within whatwg:*?

@Uzlopak
Copy link
Contributor Author

Uzlopak commented Sep 9, 2025

@metcoder95
Why though?

@metcoder95
Copy link
Member

Just wild idea, it seems that whatwg is the standard per se of these kind of tests; happy either way, but wanted to suggest it

@Uzlopak
Copy link
Contributor Author

Uzlopak commented Sep 9, 2025

@metcoder95
I am looking at the wpt folder and it seems that they dont need those prefixes, because they have basically no collisions.

Copy link
Member

@metcoder95 metcoder95 left a comment

Choose a reason for hiding this comment

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

It was more for organization, not much for collisions

@Uzlopak Uzlopak merged commit 368b527 into main Sep 10, 2025
33 of 35 checks passed
@Uzlopak Uzlopak deleted the extract-encoding-infra branch September 10, 2025 07:17
@Uzlopak
Copy link
Contributor Author

Uzlopak commented Sep 10, 2025

I understand ;)

@github-actions github-actions bot mentioned this pull request Jan 5, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants