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

fix(ses): fixup async iterator helpers #1670

Merged
merged 3 commits into from
Jul 7, 2023

Conversation

mhofman
Copy link
Contributor

@mhofman mhofman commented Jul 6, 2023

#1655 introduced support for iterator helpers, but due to a limitation in the core-js shim usage, the async iterator helper test was disabled, and the PR missed a couple errors in the implementation of permits for the async iterator helpers.

This PR fixes these errors, and addresses the testing limitations so that the iterator helpers tests (both sync and async) can behave more like normal tests.

Closes #1669
Closes #1289

@@ -146,7 +146,7 @@ export const getAnonymousIntrinsics = () => {
);
}

if (globalThis.AsyncInterator) {
if (globalThis.AsyncIterator) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Oops! Thanks for catching this.

@@ -1317,7 +1317,7 @@ export const permitted = {

// https://github.com/tc39/proposal-async-iterator-helpers
'%AsyncIteratorHelperPrototype%': {
'[[Proto]]': 'Async%IteratorPrototype%',
'[[Proto]]': '%AsyncIteratorPrototype%',
Copy link
Contributor

Choose a reason for hiding this comment

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

Oops! Thanks for catching this too.

@@ -0,0 +1,3 @@
import module from 'node:module';

module.wrapper[0] += '"use strict";';
Copy link
Contributor

Choose a reason for hiding this comment

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

Frankly, much less painful than I feared ;)

Copy link
Contributor

@erights erights left a comment

Choose a reason for hiding this comment

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

LGTM THANKS!

@mhofman
Copy link
Contributor Author

mhofman commented Jul 7, 2023

Oops! Thanks for catching this.

Well the tests caught it, but I have to admit, it was hard to diagnose because ava caught the exception and didn't print out the details (I assume because we're not using ses-ava here)

@mhofman mhofman merged commit 31298f7 into master Jul 7, 2023
12 checks passed
@mhofman mhofman deleted the mhofman/1289-fix-iterator-helpers branch July 7, 2023 00:12
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.

How to run .cjs shims strict? Anticipate new hidden intrinsics: Iteration helpers
2 participants