-
-
Notifications
You must be signed in to change notification settings - Fork 689
chore: extract infra and encoding methods #4523
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this 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/inframodule with WHATWG infra specification utilities - Creates new
lib/encodingmodule 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.
metcoder95
left a comment
There was a problem hiding this 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:*?
|
@metcoder95 |
|
Just wild idea, it seems that |
|
@metcoder95 |
metcoder95
left a comment
There was a problem hiding this 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
|
I understand ;) |
This relates to...
Rationale
Changes
Features
Bug Fixes
Breaking Changes and Deprecations
Status