diff --git a/src/bun.js/bindings/JSDOMFile.cpp b/src/bun.js/bindings/JSDOMFile.cpp index ac3e0588cf..90d602cb80 100644 --- a/src/bun.js/bindings/JSDOMFile.cpp +++ b/src/bun.js/bindings/JSDOMFile.cpp @@ -10,8 +10,56 @@ using namespace JSC; extern "C" SYSV_ABI void* JSDOMFile__construct(JSC::JSGlobalObject*, JSC::CallFrame* callframe); extern "C" SYSV_ABI bool JSDOMFile__hasInstance(EncodedJSValue, JSC::JSGlobalObject*, EncodedJSValue); -// TODO: make this inehrit from JSBlob instead of InternalFunction -// That will let us remove this hack for [Symbol.hasInstance] and fix the prototype chain. +// File.prototype inherits from Blob.prototype per the spec. +// This gives File instances all Blob methods while having a distinct prototype +// with constructor === File and [Symbol.toStringTag] === "File". +class JSDOMFilePrototype final : public JSC::JSNonFinalObject { + using Base = JSC::JSNonFinalObject; + +public: + static constexpr unsigned StructureFlags = Base::StructureFlags; + + static JSDOMFilePrototype* create(JSC::VM& vm, JSC::JSGlobalObject* globalObject, JSC::Structure* structure) + { + JSDOMFilePrototype* prototype = new (NotNull, JSC::allocateCell(vm)) JSDOMFilePrototype(vm, structure); + prototype->finishCreation(vm, globalObject); + return prototype; + } + + DECLARE_INFO; + + static JSC::Structure* createStructure(JSC::VM& vm, JSC::JSGlobalObject* globalObject, JSC::JSValue prototype) + { + auto* structure = JSC::Structure::create(vm, globalObject, prototype, JSC::TypeInfo(JSC::ObjectType, StructureFlags), info()); + structure->setMayBePrototype(true); + return structure; + } + + template + static JSC::GCClient::IsoSubspace* subspaceFor(JSC::VM& vm) + { + STATIC_ASSERT_ISO_SUBSPACE_SHARABLE(JSDOMFilePrototype, Base); + return &vm.plainObjectSpace(); + } + +protected: + JSDOMFilePrototype(JSC::VM& vm, JSC::Structure* structure) + : Base(vm, structure) + { + } + + void finishCreation(JSC::VM& vm, JSC::JSGlobalObject* globalObject) + { + Base::finishCreation(vm); + // Set [Symbol.toStringTag] = "File" so Object.prototype.toString.call(file) === "[object File]" + this->putDirectWithoutTransition(vm, vm.propertyNames->toStringTagSymbol, + jsNontrivialString(vm, "File"_s), + JSC::PropertyAttribute::DontEnum | JSC::PropertyAttribute::ReadOnly); + } +}; + +const JSC::ClassInfo JSDOMFilePrototype::s_info = { "File"_s, &Base::s_info, nullptr, nullptr, CREATE_METHOD_TABLE(JSDOMFilePrototype) }; + class JSDOMFile : public JSC::InternalFunction { using Base = JSC::InternalFunction; @@ -40,15 +88,20 @@ public: Base::finishCreation(vm, 2, "File"_s); } - static JSDOMFile* create(JSC::VM& vm, JSGlobalObject* globalObject) + static JSDOMFile* create(JSC::VM& vm, JSGlobalObject* globalObject, JSC::JSObject* filePrototype) { auto* zigGlobal = defaultGlobalObject(globalObject); auto structure = createStructure(vm, globalObject, zigGlobal->functionPrototype()); auto* object = new (NotNull, JSC::allocateCell(vm)) JSDOMFile(vm, structure); object->finishCreation(vm); - // This is not quite right. But we'll fix it if someone files an issue about it. - object->putDirect(vm, vm.propertyNames->prototype, zigGlobal->JSBlobPrototype(), JSC::PropertyAttribute::DontEnum | JSC::PropertyAttribute::DontDelete | JSC::PropertyAttribute::ReadOnly | 0); + // Set File.prototype to the distinct FilePrototype object (which inherits from Blob.prototype). + object->putDirect(vm, vm.propertyNames->prototype, filePrototype, + JSC::PropertyAttribute::DontEnum | JSC::PropertyAttribute::DontDelete | JSC::PropertyAttribute::ReadOnly); + + // Set FilePrototype.constructor = File + filePrototype->putDirect(vm, vm.propertyNames->constructor, object, + static_cast(JSC::PropertyAttribute::DontEnum)); return object; } @@ -69,7 +122,7 @@ public: auto& vm = JSC::getVM(globalObject); JSObject* newTarget = asObject(callFrame->newTarget()); auto* constructor = globalObject->JSDOMFileConstructor(); - Structure* structure = globalObject->JSBlobStructure(); + Structure* structure = globalObject->JSFileStructure(); if (constructor != newTarget) { auto scope = DECLARE_THROW_SCOPE(vm); @@ -77,7 +130,7 @@ public: // ShadowRealm functions belong to a different global object. getFunctionRealm(lexicalGlobalObject, newTarget)); RETURN_IF_EXCEPTION(scope, {}); - structure = InternalFunction::createSubclassStructure(lexicalGlobalObject, newTarget, functionGlobalObject->JSBlobStructure()); + structure = InternalFunction::createSubclassStructure(lexicalGlobalObject, newTarget, functionGlobalObject->JSFileStructure()); RETURN_IF_EXCEPTION(scope, {}); } @@ -103,9 +156,30 @@ const JSC::ClassInfo JSDOMFile::s_info = { "File"_s, &Base::s_info, nullptr, nul namespace Bun { +JSC::Structure* createJSFileStructure(JSC::VM& vm, JSC::JSGlobalObject* globalObject) +{ + auto* zigGlobal = defaultGlobalObject(globalObject); + JSC::JSObject* blobPrototype = zigGlobal->JSBlobPrototype(); + + // Create FilePrototype with [[Prototype]] = Blob.prototype + auto* protoStructure = JSDOMFilePrototype::createStructure(vm, globalObject, blobPrototype); + auto* filePrototype = JSDOMFilePrototype::create(vm, globalObject, protoStructure); + + // Create the structure for File instances: [[Prototype]] = FilePrototype + return JSC::Structure::create(vm, globalObject, filePrototype, + JSC::TypeInfo(static_cast(0b11101110), WebCore::JSBlob::StructureFlags), + WebCore::JSBlob::info(), NonArray); +} + JSC::JSObject* createJSDOMFileConstructor(JSC::VM& vm, JSC::JSGlobalObject* globalObject) { - return JSDOMFile::create(vm, globalObject); + auto* zigGlobal = defaultGlobalObject(globalObject); + + // Get the File instance structure - its prototype is the FilePrototype we need + auto* fileStructure = zigGlobal->JSFileStructure(); + auto* filePrototype = fileStructure->storedPrototypeObject(); + + return JSDOMFile::create(vm, globalObject, filePrototype); } } diff --git a/src/bun.js/bindings/JSDOMFile.h b/src/bun.js/bindings/JSDOMFile.h index eed1e98ab8..28f605661e 100644 --- a/src/bun.js/bindings/JSDOMFile.h +++ b/src/bun.js/bindings/JSDOMFile.h @@ -4,4 +4,5 @@ namespace Bun { JSC::JSObject* createJSDOMFileConstructor(JSC::VM&, JSC::JSGlobalObject*); +JSC::Structure* createJSFileStructure(JSC::VM&, JSC::JSGlobalObject*); } diff --git a/src/bun.js/bindings/ZigGlobalObject.cpp b/src/bun.js/bindings/ZigGlobalObject.cpp index 31373b7359..323f770d48 100644 --- a/src/bun.js/bindings/ZigGlobalObject.cpp +++ b/src/bun.js/bindings/ZigGlobalObject.cpp @@ -1805,6 +1805,11 @@ void GlobalObject::finishCreation(VM& vm) init.set(CustomGetterSetter::create(init.vm, errorInstanceLazyStackCustomGetter, errorInstanceLazyStackCustomSetter)); }); + m_JSFileStructure.initLater( + [](const Initializer& init) { + init.set(Bun::createJSFileStructure(init.vm, init.owner)); + }); + m_JSDOMFileConstructor.initLater( [](const Initializer& init) { JSObject* fileConstructor = Bun::createJSDOMFileConstructor(init.vm, init.owner); diff --git a/src/bun.js/bindings/ZigGlobalObject.h b/src/bun.js/bindings/ZigGlobalObject.h index 21c8d41f5c..087252a690 100644 --- a/src/bun.js/bindings/ZigGlobalObject.h +++ b/src/bun.js/bindings/ZigGlobalObject.h @@ -610,6 +610,7 @@ public: V(private, LazyPropertyOfGlobalObject, m_importMetaBakeObjectStructure) \ V(private, LazyPropertyOfGlobalObject, m_asyncBoundFunctionStructure) \ V(public, LazyPropertyOfGlobalObject, m_JSDOMFileConstructor) \ + V(public, LazyPropertyOfGlobalObject, m_JSFileStructure) \ V(public, LazyPropertyOfGlobalObject, m_JSMIMEParamsConstructor) \ V(public, LazyPropertyOfGlobalObject, m_JSMIMETypeConstructor) \ \ @@ -712,6 +713,7 @@ public: JSObject* cryptoObject() const { return m_cryptoObject.getInitializedOnMainThread(this); } JSObject* JSDOMFileConstructor() const { return m_JSDOMFileConstructor.getInitializedOnMainThread(this); } + JSC::Structure* JSFileStructure() const { return m_JSFileStructure.getInitializedOnMainThread(this); } JSMap* nodeWorkerEnvironmentData() { return m_nodeWorkerEnvironmentData.get(); } void setNodeWorkerEnvironmentData(JSMap* data); diff --git a/test/regression/issue/26899.test.ts b/test/regression/issue/26899.test.ts new file mode 100644 index 0000000000..04b3e2b311 --- /dev/null +++ b/test/regression/issue/26899.test.ts @@ -0,0 +1,67 @@ +import { expect, test } from "bun:test"; + +// https://github.com/oven-sh/bun/issues/26899 +// File.prototype should be distinct from Blob.prototype + +test("File.prototype !== Blob.prototype", () => { + expect(File.prototype).not.toBe(Blob.prototype); +}); + +test("File.prototype inherits from Blob.prototype", () => { + expect(Object.getPrototypeOf(File.prototype)).toBe(Blob.prototype); +}); + +test("new File(...).constructor.name === 'File'", () => { + const file = new File(["hello"], "hello.txt"); + expect(file.constructor.name).toBe("File"); +}); + +test("new File(...).constructor === File", () => { + const file = new File(["hello"], "hello.txt"); + expect(file.constructor).toBe(File); +}); + +test("new File(...).constructor !== Blob", () => { + const file = new File(["hello"], "hello.txt"); + expect(file.constructor).not.toBe(Blob); +}); + +test("Object.prototype.toString.call(file) === '[object File]'", () => { + const file = new File(["hello"], "hello.txt"); + expect(Object.prototype.toString.call(file)).toBe("[object File]"); +}); + +test("file instanceof File", () => { + const file = new File(["hello"], "hello.txt"); + expect(file instanceof File).toBe(true); +}); + +test("file instanceof Blob", () => { + const file = new File(["hello"], "hello.txt"); + expect(file instanceof Blob).toBe(true); +}); + +test("blob is not instanceof File", () => { + const blob = new Blob(["hello"]); + expect(blob instanceof File).toBe(false); +}); + +test("File instances have Blob methods", () => { + const file = new File(["hello"], "hello.txt"); + expect(typeof file.text).toBe("function"); + expect(typeof file.arrayBuffer).toBe("function"); + expect(typeof file.slice).toBe("function"); + expect(typeof file.stream).toBe("function"); +}); + +test("File name and lastModified work", () => { + const file = new File(["hello"], "hello.txt", { lastModified: 12345 }); + expect(file.name).toBe("hello.txt"); + expect(file.lastModified).toBe(12345); +}); + +test("File.prototype has correct Symbol.toStringTag", () => { + const desc = Object.getOwnPropertyDescriptor(File.prototype, Symbol.toStringTag); + expect(desc).toBeDefined(); + expect(desc!.value).toBe("File"); +});