Skip to content

Implement Promise.allKeyed, Promise.allSettledKeyed.#330

Merged
Jack-Works merged 9 commits intoengine262:mainfrom
ajvincent:Promise.allKeyed
Mar 31, 2026
Merged

Implement Promise.allKeyed, Promise.allSettledKeyed.#330
Jack-Works merged 9 commits intoengine262:mainfrom
ajvincent:Promise.allKeyed

Conversation

@ajvincent
Copy link
Copy Markdown
Contributor

Smoketest on npm run inspector:

const blocking = Promise.withResolvers();
async function getShape() {
  await blocking.promise;
  return "cube";
}

async function getColor() {
  await blocking.promise;
  throw new Error("no color available");
}

async function getMass() {
  await blocking.promise;
  return 4000; // kg
}

const promiseDictionary = {
  shape: getShape(),
  color: getColor(),
  mass: getMass(),
};

const dictionary = Promise.allSettledKeyed(promiseDictionary);
blocking.resolve();

try {
  console.log(await dictionary);
} catch (ex) {
  console.log("caught exception");
  console.error(ex);
}

I was getting reasonable results when I tweaked this for all promises resolving, and for allSettled instead of allSettledKeyed.

As directed, this is behind a feature flag. Nice that npm run inspector picked up on it automatically.


/** https://tc39.es/proposal-await-dictionary/#sec-promise.allkeyed */
function* Promise_allKeyed([promises = Value.undefined]: Arguments, { thisValue }: FunctionCallContext): ValueEvaluator {
// 1. Let C be the this value.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Hi! I would prefer to remove spec steps if they're very straitforward.

For example I will keep comments like this:

// 1. If an implementation-defined debugging facility is available and enabled, then
  // 1. If StringToCodePoints(text) is not a valid JSON text as specified in ECMA-404, throw a SyntaxError exception.
  Q(JSONValidator.validate(text));

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I know... I have this bad habit of being robotic, trying to capture everything as written, when it comes to the specs.

@Jack-Works
Copy link
Copy Markdown
Contributor

Hi, I just pushed a big commit, if you have difficulties with merging plz let me know

@ajvincent
Copy link
Copy Markdown
Contributor Author

Hi, I just pushed a big commit, if you have difficulties with merging plz let me know

npm run inspector lost the feature flags.

Uncaught TypeError: Cannot destructure property 'Agent' of 'self.@engine262/engine262' as it is undefined.
at worker.js:9:3

@Jack-Works
Copy link
Copy Markdown
Contributor

Hi, I just pushed a big commit, if you have difficulties with merging plz let me know

npm run inspector lost the feature flags.

Uncaught TypeError: Cannot destructure property 'Agent' of 'self.@engine262/engine262' as it is undefined. at worker.js:9:3

Hi I cannot reproduce this on my side, have you update the submodule (website folder)?


// 7. Return undefined.
return Value.undefined;
})(index);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This looks like a spec bug. Spec capture should not (at least no precedence, as I can recall) capture a variable (only constants).

You can raise an issue in the proposal.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@Jack-Works
Copy link
Copy Markdown
Contributor

Hi! Please revert your changes on website and test262 submodule (unless it is necessary).

@ajvincent
Copy link
Copy Markdown
Contributor Author

Hi! Please revert your changes on website and test262 submodule (unless it is necessary).

This was an unintended change. I'm trying to find the time to do the necessary cleanup.

ajvincent and others added 3 commits March 12, 2026 20:51
@Jack-Works Jack-Works merged commit fd20e2b into engine262:main Mar 31, 2026
2 checks passed
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.

2 participants