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

Implement ES2024 Promise.withResolvers #1452

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

robik
Copy link
Contributor

@robik robik commented Jun 28, 2024

Summary

This PR implements ES2024 Promise.withResolvers function.

Test Plan

Added new test

@facebook-github-bot facebook-github-bot added the CLA Signed Do not delete this pull request or issue due to inactivity. label Jun 28, 2024
@tmikov
Copy link
Contributor

tmikov commented Jun 28, 2024

Hmm, we have to figure out how to deal with this. The problem is that 01-Promise.js is automatically generated by https://github.com/facebook/hermes/blob/main/utils/gen-promise-internal-bc.sh from https://www.npmjs.com/package/promise.

However, at the same time, we can't be stuck forever with an outdated Promise implementation. The discussion in #1009 is also relevant here.

So, the question is, should we:

  1. Essentially fork https://github.com/then/promise, which hasn't been updated in some time and start updating it?
  2. Extract the promise from https://www.npmjs.com/package/es6-shim. That carries the risk of being incompatible with React Native (unfortunately the RN team hasn't clarified the extent to which they depend on non-spec behavior of the current polyfill).

I think that at least in the short term 1) is the only practical option. Then the question becomes, how to actually modify it? Should we just tweak the built result? (I am on vacation, so I may be slow to respond, but I am curious what others think too)

@neildhar
Copy link
Contributor

neildhar commented Jul 3, 2024

Forking it does seem like the most straightforward. I suppose the best way to do it comes down to how difficult it is to build the promise implementation. If we could easily copy the original source to our external directory, and point utils/promise to it, then we can freely modify the original input while maintaining its structure. That has some value if we find ourselves digging into the blame for the implementation in the future, or if there is some movement in the repo upstream.

@ljharb
Copy link

ljharb commented Jul 3, 2024

@tmikov still happy to extract the Promise implementation to a separate package, if that'd be helpful - i could include finally and withResolvers in it, since those are already authored as standalone packages. (i can also help you do an optimal build to eliminate parts of the graph that aren't needed in hermes)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed Do not delete this pull request or issue due to inactivity.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants