Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
feat(ses): Shim compatible with Hermes compiler #2334
base: master
Are you sure you want to change the base?
feat(ses): Shim compatible with Hermes compiler #2334
Changes from all commits
be88da5
fa68e56
8c7e6d1
1cdbea8
fa520ca
58dc9a4
e3cbe03
3137c9a
f35f394
43aece9
cad46eb
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
Please add at least a comment or a type that makes clear the conditional nature of this export. Maybe even a rename to imply its conditional nature?
Since this export is exported static state that isn't entered as an intrinsic and so not hardened by lockdown, should we freeze it here before exporting? Is there any other non
FERAL_...
values exported by commons.js but untouched by lockdown that should be frozen before export?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.
Just checking: We do not have any such problem with async functions or generator functions? Only with async generator functions?
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.
yes and we have a problemo with async arrow functions, it's abit of a mess, so i'll clarify:
generators are supported (correct docs)
async functions are supported (comment)
despite the error in the codebase
despite listed in docs as In Progress
it's actually async arrow functions aren't supported (not in docs/code) so i filed the issue
(fixed since, but only in unreleased Static Hermes)
so we're handling these on Hermes in this PR packages/ses/scripts/hermes-transforms.js
for a Hermes flavoured SES shim
and then finally yes async generators are unsupported is accurate (but not mentioned in docs)
so we're handling that here with
AsyncGeneratorFunctionInstance
delaying the error till runtimethen repairing and hardening Hermes accordingly