Skip to content
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

stream: treat null asyncIterator as undefined #55119

Merged
merged 1 commit into from
Sep 27, 2024

Conversation

jazelly
Copy link
Contributor

@jazelly jazelly commented Sep 25, 2024

According to the spec, getIterator should normalize incoming method to
undefined if it is either undefined or null. This PR enforces that spec
compliance with passed WPT.

@nodejs-github-bot
Copy link
Collaborator

Review requested:

  • @nodejs/web-standards

@nodejs-github-bot nodejs-github-bot added needs-ci PRs that need a full CI run. web streams labels Sep 25, 2024
Copy link
Member

@KhafraDev KhafraDev left a comment

Choose a reason for hiding this comment

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

According to the spec, getIterator should normalize incoming method to
undefined if it is either undefined or null.

Where is this? The tc39 link in the PR specifically does not mentioned null?

Let method be ? [GetMethod](https://tc39.es/ecma262/#sec-getmethod)(obj, [%Symbol.asyncIterator%](https://tc39.es/ecma262/#sec-well-known-symbols)).
   b. If method is undefined, then

          i. Let syncMethod be ? [GetMethod](https://tc39.es/ecma262/#sec-getmethod)(obj, [%Symbol.iterator%](https://tc39.es/ecma262/#sec-well-known-symbols)).
          ii. If syncMethod is undefined, throw a TypeError exception.

edit: it's in GetMethod:

If func is either undefined or null, return undefined.

@jakecastelli
Copy link
Contributor

It is in the getMethod

If func is either undefined or null, return undefined.

Copy link

codecov bot commented Sep 25, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 88.24%. Comparing base (5c22d19) to head (0a4ffe8).
Report is 30 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #55119      +/-   ##
==========================================
- Coverage   88.25%   88.24%   -0.01%     
==========================================
  Files         651      651              
  Lines      183856   183858       +2     
  Branches    35850    35854       +4     
==========================================
- Hits       162259   162248      -11     
- Misses      14888    14893       +5     
- Partials     6709     6717       +8     
Files with missing lines Coverage Δ
lib/internal/webstreams/util.js 99.65% <100.00%> (+<0.01%) ⬆️

... and 38 files with indirect coverage changes

According to the spec, getIterator should normalize incoming method to
undefined if it is either undefined or null. This PR enforces that spec
compliance with passed WPT.
@KhafraDev KhafraDev added author ready PRs that have at least one approval, no pending requests for changes, and a CI started. request-ci Add this label to start a Jenkins CI on a PR. labels Sep 26, 2024
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Sep 26, 2024
@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

@KhafraDev KhafraDev added the commit-queue Add this label to land a pull request using GitHub Actions. label Sep 26, 2024
@nodejs-github-bot nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Sep 27, 2024
@nodejs-github-bot nodejs-github-bot merged commit 87a79cd into nodejs:main Sep 27, 2024
58 checks passed
@nodejs-github-bot
Copy link
Collaborator

Landed in 87a79cd

targos pushed a commit that referenced this pull request Oct 4, 2024
According to the spec, getIterator should normalize incoming method to
undefined if it is either undefined or null. This PR enforces that spec
compliance with passed WPT.

PR-URL: #55119
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Matthew Aitken <maitken033380023@gmail.com>
Reviewed-By: Mattias Buelens <mattias@buelens.com>
targos pushed a commit that referenced this pull request Oct 4, 2024
According to the spec, getIterator should normalize incoming method to
undefined if it is either undefined or null. This PR enforces that spec
compliance with passed WPT.

PR-URL: #55119
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Matthew Aitken <maitken033380023@gmail.com>
Reviewed-By: Mattias Buelens <mattias@buelens.com>
@aduh95 aduh95 mentioned this pull request Oct 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
author ready PRs that have at least one approval, no pending requests for changes, and a CI started. needs-ci PRs that need a full CI run. web streams
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants