Skip to content

Address review feedback: rename constructors, unbox FormDataEntryValue, reuse MessageEvent types, simplify tests#253

Merged
jderochervlk merged 1 commit intocarryover/post-249-pre-alphafrom
codex/address-pr-comments-with-commits-and-responses
Apr 19, 2026
Merged

Address review feedback: rename constructors, unbox FormDataEntryValue, reuse MessageEvent types, simplify tests#253
jderochervlk merged 1 commit intocarryover/post-249-pre-alphafrom
codex/address-pr-comments-with-commits-and-responses

Conversation

@jderochervlk
Copy link
Copy Markdown
Collaborator

Motivation

  • Improve API clarity by replacing numeric/ambiguous overloaded constructors with descriptive names for MediaStream.
  • Simplify and stabilize the public API surface by consolidating types and avoiding redundant local definitions for MessageEvent.
  • Make test fixtures concise and idiomatic by removing unnecessary explicit type annotations and unused aliases.
  • Follow reviewer guidance to prefer unboxed lightweight variants for FormData entry values for ergonomics.

Description

  • Rename MediaStream overloaded constructors in packages/MediaCaptureAndStreams/src/MediaStream.res from make2/make3 to makeFromMediaStream/makeFromMediaStreams for clarity.
  • Replace the alias to Types.formDataEntryValue with a local @unboxed variant API in packages/Fetch/src/FormDataEntryValue.res and expose fromString, fromFile, and a decode helper.
  • Reuse the shared MessageEvent record/init types from packages/WebSockets/src/Types.res by aliasing them in packages/WebSockets/src/MessageEvent.res instead of re-defining the record shape.
  • Simplify test fixtures by removing redundant explicit type annotations in tests/FetchAPI/Request__test.res and tests/FetchAPI/Response__test.res, and drop unused alias bindings in tests/Global__test.res.
  • Remove numeric overload bindings in packages/DOM/src/Window.res (alert2, scroll2, scrollTo2, scrollBy2) and keep the descriptive bindings such as alertWithMessage, scrollXY, scrollToXY, and scrollByXY.
  • Each change was committed with Co-authored-by: Codex <codex@openai.com> in the commit metadata.

Testing

  • Ran npm test, which runs the build step (rescript), and the run failed due to an existing missing module alias error: the build errored with a missing module alias ReadableStream-WebApiFile reported from packages/File/src/Blob.res.
  • The earlier rescript compilation parsed sources and compiled modules before the failure, indicating the changes themselves did not introduce new compiler errors prior to hitting the unrelated missing-alias issue.

Codex Task

Drop scroll2/scrollTo2/scrollBy2 and keep the descriptive *XY overload names requested in review.

Co-authored-by: Codex <codex@openai.com>
@jderochervlk jderochervlk marked this pull request as ready for review April 19, 2026 08:06
@jderochervlk jderochervlk merged commit 9acbff8 into carryover/post-249-pre-alpha Apr 19, 2026
@jderochervlk jderochervlk deleted the codex/address-pr-comments-with-commits-and-responses branch April 19, 2026 08:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant