Normative: make Promise.try use PromiseResolve in non-error case - #3883
Merged
Merged
Conversation
|
The rendered spec preview for this PR is available as a single page at https://tc39.es/ecma262/pr/3883 and as multiple pages at https://tc39.es/ecma262/pr/3883/multipage . |
nicolo-ribaudo
approved these changes
Jul 16, 2026
zloirock
added a commit
to zloirock/core-js
that referenced
this pull request
Jul 25, 2026
3 tasks
TeakWood
added a commit
to TeakWood/eslint-shreni
that referenced
this pull request
Aug 8, 2026
…existing) (eslint-shreni-beads-edl)
The failing job is NOT a regression in this build -- it is a break in one of eslint-plugin-unicorn's own dependencies.
Diagnosis: I reproduced the failure locally and found exactly one failing ava assertion out of the whole unicorn suite: `no-unnecessary-polyfills > invalid(6): require("core-js/stable/promise") Should have 1 error but had 0: []`. core-js-compat 3.50.0, released 2026-08-05, emptied the support data for `es.promise.try` and `esnext.promise.try` following tc39/ecma262#3883, so that polyfill now counts as needed on every target and the plugin's test that expects one error gets none. I confirmed the mechanism directly in the sandbox: core-js-compat resolved to 3.50.0 and both entries were `{}`. The plugin's dependency range is `^3.49.0`, so moving its commit pin forward does not avoid it. This also explains the "failed on EVERY run on main" pattern -- this fork's first ecosystem run post-dates the 3.50.0 release.
Fix: adopted the configuration upstream verified in eslint#21191 -- commit pin dd52b4a4 with `npm install core-js-compat@3.49.0`. Verified end to end from a clean sandbox against this fork's ESLint: `Passed: eslint-plugin-unicorn`, exit 0. I confirmed the pin survives the subsequent `npm install --no-save <local eslint>` (core-js-compat stays at 3.49.0 and `es.promise.try.node` is back to "23.0").
Second, I fixed why this took an upstream-PR dig to diagnose at all. The bead's own next step -- "pull the eslint-plugin-unicorn test:js output from the run" -- was impossible, because the harness built its error from stderr alone and discarded stdout. The CI log carried only `ERROR: "test:js" exited with 1.`: the failing script named, and not one failing assertion. The new describeCommandFailure() includes the tail of both streams. It also fixes a latent silent-pass bug in the same three lines: `if (result.status || result.error)` treats `status === null` as success, so a test runner killed by a signal came back green.
All four mutations of the new guards fail as intended: reverting the index.mjs wiring (1 failure), dropping the core-js-compat pin (1), dropping the stdout section (4), and restoring the truthiness test on `status` (2). Working tree restored and verified after each.
Gates, all run against the committed tree: `pnpm lint:types` exit 0, `pnpm lint` exit 0, `pnpm test` exit 0 (38,829 passing / 11 pending / 0 failing), `pnpm test:coverage` exit 0 (99.31% statements, 98.31% branches -- above the 99/98 floor). Working tree clean.
Confidence: 92%
Files changed: 5
Questions for reviewer:
- I moved the unicorn commit pin forward (a33b1c1e -> dd52b4a4) rather than keeping ours and adding the core-js-compat pin alongside the existing `eslint-plugin-eslint-plugin@7.5.0` one. The reason is a flag conflict: core-js-compat is a production dependency of unicorn and eslint-plugin-eslint-plugin is a devDependency, so pinning both in the single `install` command the schema allows would have moved one across dependency groups -- which unicorn's own `lint:package-json` step runs against during `npm test`. Adopting upstream's exact configuration sidesteps that, is what upstream has verified green, and I also verified it green locally against this fork. Is moving the commit pin acceptable in a bug-fix bead, or would you rather I kept a33b1c1e and extended the schema to allow multiple install commands?
- The 'pins core-js-compat for eslint-plugin-unicorn' test asserts a specific version string, so it fails loudly if someone drops the pin -- but it will not fail once sindresorhus/eslint-plugin-unicorn#3606 lands and the pin becomes unnecessary; it will just quietly keep passing. I recorded the retirement condition in the test comment and the README rather than trying to make it self-expiring. Is that the right trade here?
- I could not verify the CI job itself is green -- only the local end-to-end run. Sthapathi should confirm the post-merge 'Test Ecosystem Plugins' run.
lahma
added a commit
to sebastienros/jint
that referenced
this pull request
Aug 11, 2026
…2978) The range b363f29d..3655e746 is five commits: three CI-only, the [object-rest] comma-after-rest parse tests (already passing), and tests for the faster-Promise.try normative change. That last one is a real behaviour change. tc39/ecma262#3883 ("make Promise.try use PromiseResolve in non-error case") has consensus and its test262 coverage has landed, so Promise.try now hands the callback's result to PromiseResolve on the normal path -- returning an already-matching promise as-is instead of wrapping it -- and builds a capability only on the abrupt path. Note that neither the living spec nor the (frozen) proposal page renders this yet; the tests at the pinned SHA are what governs. Reordering those steps exposed two latent bugs on paths the old ordering could not reach: - Promise.try() with no arguments escaped as a raw CLR ArrayTypeMismatchException. ExpressionCache.ArgumentListEvaluation hands the callee Unsafe.As<JsValue[]>(object?[]) whenever the argument list is fully cached, which the empty list vacuously is, so arguments.AsSpan() throws. arguments.Skip(1) copies element-wise and returns [] when there is nothing to take. - JsValue.Call on a non-callable raises a CLR ArgumentException rather than a JS TypeError, so a missing callback could not become the rejection Completion(Call(...)) calls for. Going through GetCallable fixes that. Neither is covered by test262, so both get a regression test. Also adds a repository skill capturing the update procedure: how to triage a range (ancestry, not author date), how to find a normative change when the rendered spec is behind, and the argument-handling traps above. test262: 0 failed, 99744 passed (+6, the three new Promise.try tests in both modes). Jint.Tests and Jint.Tests.PublicInterface green on net10.0 and net472. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
webkit-commit-queue
pushed a commit
to Constellation/WebKit
that referenced
this pull request
Aug 17, 2026
https://bugs.webkit.org/show_bug.cgi?id=321890 rdar://185064567 Reviewed by Sosuke Suzuki. Test262 is updated[1] to include the latest spec change proposal for Promise.try[2], which uses PromiseResolve instead of a promise created by NewPromiseCapability. This simplifies our Promise.try implementation since we can just use @promiseResolve and @promisereject helpers instead. Also this removes the reference to @newPromiseCapability in JSC code. So we drop PromiseOperations.js and its JS builtins, and replace @newPromiseCapability with C++ version for remaining WebCore users. WebCore users can be replaced by using normal Promise + first-resolving handler. This is fixed in a subsequent change. [1]: tc39/test262#5072 [2]: tc39/ecma262#3883 * JSTests/test262/expectations.yaml: * Source/JavaScriptCore/CMakeLists.txt: * Source/JavaScriptCore/DerivedSources-input.xcfilelist: * Source/JavaScriptCore/DerivedSources.make: * Source/JavaScriptCore/JavaScriptCore.xcodeproj/project.pbxproj: * Source/JavaScriptCore/builtins/BuiltinNames.h: * Source/JavaScriptCore/builtins/PromiseConstructor.js: (try): * Source/JavaScriptCore/builtins/PromiseOperations.js: Removed. * Source/JavaScriptCore/bytecode/LinkTimeConstant.h: * Source/JavaScriptCore/runtime/JSGlobalObject.cpp: (JSC::JSC_DEFINE_HOST_FUNCTION): (JSC::JSGlobalObject::init): Canonical link: https://commits.webkit.org/319276@main
hubot
pushed a commit
to v8/v8
that referenced
this pull request
Aug 18, 2026
Update Promise.try to align with normative TC39 ecma262 PR tc39/ecma262#3883 In the non-throwing case of Promise.try we just return that Promise directly instead of wrapping it in a new one. The new behavior is behind a shipping runtime flag --js-pr-3883. TAG=agy CONV=1a52772a-fdcf-40b0-b760-9c4d5681a991 Bug: 547302828 Change-Id: I7346553be32998580742a1c6e55ee497fddafd0d Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/8251141 Reviewed-by: Nikolaos Papaspyrou <nikolaos@chromium.org> Commit-Queue: Olivier Flückiger <olivf@chromium.org> Auto-Submit: Olivier Flückiger <olivf@chromium.org> Cr-Commit-Position: refs/heads/main@{#109304}
This was referenced Aug 22, 2026
linusg
approved these changes
Aug 27, 2026
michaelficarra
approved these changes
Aug 27, 2026
ljharb
force-pushed
the
faster-promise-try
branch
from
August 28, 2026 17:38
630d8b7 to
9235b15
Compare
This was referenced Sep 1, 2026
Jack-Works
added a commit
to engine262/engine262
that referenced
this pull request
Sep 6, 2026
ljharb
added a commit
to es-shims/Promise.try
that referenced
this pull request
Sep 9, 2026
tc39/ecma262#3883 changed `Promise.try` to end with `Return ? PromiseResolve(ctor, ! status)`, instead of creating a capability up front and resolving through it. A promise returned from the callback is now passed through rather than wrapped, and the capability is only created on the abrupt-completion path. test262 pins this in `built-ins/Promise/try/avoids-wrap.js` and `avoids-wrap-for-subclass.js` (tc39/test262#5072). No engine has shipped the change yet, so `getPolyfill` probes for the older wrapping behavior and falls back to the implementation.
ljharb
added a commit
to es-shims/Promise.try
that referenced
this pull request
Sep 9, 2026
tc39/ecma262#3883 changed `Promise.try` to end with `Return ? PromiseResolve(ctor, ! status)`, instead of creating a capability up front and resolving through it. A promise returned from the callback is now passed through rather than wrapped, and the capability is only created on the abrupt-completion path. test262 pins this in `built-ins/Promise/try/avoids-wrap.js` and `avoids-wrap-for-subclass.js` (tc39/test262#5072). No engine has shipped the change yet, so `getPolyfill` probes for the older wrapping behavior and falls back to the implementation.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Using PromiseResolve differs from the current behavior in that when the argument returns a Promise whose
.constructoris whatever thethisvalue of thePromise.trycall is1, we just return that Promise directly instead of wrapping it in a new one. Otherwise it's exactly the same.This has the advantage of better matching the intuition that this is for cases where you want to write
Promise.resolve(foo())but you also want to wrap sync errors in rejected Promises. Now it actually works exactly likeAs a bonus, it also means this costs zero microtask ticks in the case where the argument is actually an async function, instead of (as currently) two.
See matrix chat 1 2.
Footnotes
Precise check might change. ↩