Repository navigation
async function should return default promise in node:fs/promises - #106
Merged
Jarred-Sumner merged 2 commits intoAug 28, 2025
Merged
Conversation
sosukesuzuki
marked this pull request as ready for review
August 28, 2025 02:04
Jarred-Sumner
requested changes
Aug 28, 2025
Jarred-Sumner
left a comment
Jarred-Sumner
left a comment
Collaborator
There was a problem hiding this comment.
node:fs/promises shouldn't be special cased here.
Member
Author
If there are no use cases where we want to use JSInternalPromise in Bun's built-ins, sounds good. |
Co-authored-by: Jarred Sumner <jarred@jarredsumner.com>
sosukesuzuki
force-pushed
the
fs-promises-should-not-return-internal-promise
branch
from
August 28, 2025 02:08
f51364c to
f987d1e
Compare
Jarred-Sumner
approved these changes
Aug 28, 2025
steipete
added a commit
to openclaw/WebKit
that referenced
this pull request
Oct 4, 2026
Make JSC stack positions select the same syntax tokens as Node 24: call and constructor starts, property reads, and async `await` continuations. Keep those positions separate from exception-expression divots and debugger line tables. Both captured `StackFrame` and live `StackVisitor` readers use the metadata; executable line overrides and release-private builtin position policy are preserved. The two sparse vectors are owned by each unlinked code block, remapped through bytecode rewriting/optimization, and decoded into owned storage on both cache paths. The cache revision advances with the metadata. Property reads and calls retain distinct tokens, so `(null.x)()` can fail at `x` before reaching the call. The paired Bun adapter in [openclaw/bun#114](openclaw/bun#114) also preserves error constructors and runtime call syntax. Rewriting `new Error()` into `Error()` moved the reported column and made a returning arrow eligible for JSC proper-tail-call elision. Preserving `new` restores the frame without disabling tail calls. Callee parentheses, computed access, and opening delimiter mappings are required for the broader normal-transpile corpus; TypeScript generic/non-null suffixes and cache invalidation are covered. This continues the source-position work in [oven-sh/bun#35179](oven-sh/bun#35179), [WebKit#37396](oven-sh/bun#37396), and [WebKit#41580](oven-sh/bun#41580) by @robobun. Native Linux qualification on one c7a.24xlarge host, with the unchanged W113 lane/Docker/ICU recipe: - 1,509,853 FFI checks; 121 JSC stress configurations; interpreter/JIT/bytecode-optimizer/eager-FTL modes; owned and persistent cold/warm caches. Each cache path creates three files totaling 33,024 bytes. - Raw JSC positions improve from 13/30 to 30/30 in the call corpus and from 4/28 to 28/28 in the added cases. The live visitor matches Node at 1:22, 2:21, 3:1. Async continuation control passes. - Paired Bun: 47/47 fork-selection results with zero regressions, 343 CallSite/util/source-map tests, and 68 minifier tests. Normal transpilation matches all 30 call shapes, all 28 added cases on both stack surfaces, and 14 TypeScript/generic/non-null/astral cases. The JavaScript/TypeScript module corpus and 58 raw-eval rows match as well. - Runtime cache replacement and warm replay match all 56 added observations. Existing custom-stack generated-versus-mapped and filename/eval-origin policies are preserved. - OpenClaw loader pair 8/8, SDK under its existing native-loader policy 67 pass/1 skip, Slack ordered shared-worker sequence 26/26, and real Proxyline probe pass. The original shared SDK policy retains its same two baseline tsconfig resolver failures. - ABBA, eight samples per arm: exception creation/formatting +7.6%; fresh 350-module startup 0.998× baseline. The exception overhead is an explicit tradeoff. - Final complete engine and Bun branch reviews are scoped-clean through P2. Exact-head [CI run 37240058657](https://github.com/openclaw/WebKit/actions/runs/37240058657) is green at `6e69d757023aa75398a670f487f90fe2a57e2b99`. Qualification caught and corrected a namespace-only result-checker assumption, repeated C++ default arguments in unity builds, private-builtin position leakage, missing transpiler delimiter metadata, TypeScript non-null suffix tracking, and the omitted live-stack reader. Exact position goldens were checked against Node; the original astral inline-snapshot assertion remains intact and passes. The Bun draft remains stacked on manifest commit `cf636c2f14914b3ba4b319874312d654cc5ea064` and also needs the namespace adapter from Bun oven-sh#106. Native baseline and candidate both include that adapter because current engine main requires its API. This PR does not publish artifacts or change the artifact recipe; batch publication stays with the coordinator.
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
2 participants
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.
Fixes oven-sh/bun#22167
JSC generates bytecode so that async functions defined within built-in JS code return
JSInternalPromiseinstead of the default Promise. This is intentional, but this caused Bun's node:fs/promises exported functions to incorrectly returnJSInternalPromise.This patch changes it to return the appropriate Promise by checking the source URL where the built-in is defined.