From 2834abecb558b9c06d09524d154535a5f2782caa Mon Sep 17 00:00:00 2001 From: pfg Date: Thu, 19 Dec 2024 17:41:54 -0800 Subject: [PATCH] infer module type in create & switch some to Module --- src/bake/BakeGlobalObject.cpp | 2 +- src/bake/BakeSourceProvider.cpp | 2 +- .../bindings/BunAnalyzeTranspiledModule.cpp | 3 -- src/bun.js/bindings/CommonJSModuleRecord.cpp | 4 +-- src/bun.js/bindings/ModuleLoader.cpp | 6 ++-- src/bun.js/bindings/ZigSourceProvider.cpp | 10 ++++++- src/bun.js/bindings/ZigSourceProvider.h | 1 - src/bun.js/bindings/bindings.cpp | 2 +- src/bun.js/bindings/exports.zig | 2 -- src/bun.js/bindings/headers-handwritten.h | 1 - src/bun.js/bindings/napi.cpp | 2 +- src/bun.js/module_loader.zig | 29 ++++++++++++------- 12 files changed, 36 insertions(+), 28 deletions(-) diff --git a/src/bake/BakeGlobalObject.cpp b/src/bake/BakeGlobalObject.cpp index 5c1ccac907..4bd4a4647e 100644 --- a/src/bake/BakeGlobalObject.cpp +++ b/src/bake/BakeGlobalObject.cpp @@ -115,7 +115,7 @@ JSC::JSInternalPromise* bakeModuleLoaderFetch(JSC::JSGlobalObject* globalObject, origin, WTFMove(moduleKey), WTF::TextPosition(), - JSC::SourceProviderSourceType::BunTranspiledModule)); + JSC::SourceProviderSourceType::Module)); return resolvedInternalPromise(globalObject, JSC::JSSourceCode::create(vm, WTFMove(sourceCode))); } return rejectedInternalPromise(globalObject, createTypeError(globalObject, makeString("Bundle does not have \""_s, moduleKey, "\". This is a bug in Bun's bundler."_s))); diff --git a/src/bake/BakeSourceProvider.cpp b/src/bake/BakeSourceProvider.cpp index 0a0970b4e4..2a51ffa0aa 100644 --- a/src/bake/BakeSourceProvider.cpp +++ b/src/bake/BakeSourceProvider.cpp @@ -122,7 +122,7 @@ extern "C" JSC::EncodedJSValue BakeRegisterProductionChunk(JSC::JSGlobalObject* origin, WTFMove(string), WTF::TextPosition(), - JSC::SourceProviderSourceType::BunTranspiledModule + JSC::SourceProviderSourceType::Module )); global->moduleLoader()->provideFetch(global, key, sourceCode); diff --git a/src/bun.js/bindings/BunAnalyzeTranspiledModule.cpp b/src/bun.js/bindings/BunAnalyzeTranspiledModule.cpp index 97f0d5bb81..af6290728d 100644 --- a/src/bun.js/bindings/BunAnalyzeTranspiledModule.cpp +++ b/src/bun.js/bindings/BunAnalyzeTranspiledModule.cpp @@ -192,9 +192,6 @@ extern "C" EncodedJSValue Bun__analyzeTranspiledModule(JSGlobalObject* globalObj VariableEnvironment lexicalVariables = VariableEnvironment(); auto provider = static_cast(sourceCode.provider()); - if (provider->m_resolvedSource.__is_probably_real != 0xABEF) { - RELEASE_AND_RETURN(scope, JSValue::encode(rejectWithError(createError(globalObject, WTF::String::fromLatin1("resolvedSource !isProbablyReal"))))); - } if (provider->m_resolvedSource.module_info == nullptr) { dataLog("[note] module_info is null for module: ", moduleKey.utf8(), "\n"); diff --git a/src/bun.js/bindings/CommonJSModuleRecord.cpp b/src/bun.js/bindings/CommonJSModuleRecord.cpp index e1d7e0573b..44cb17edac 100644 --- a/src/bun.js/bindings/CommonJSModuleRecord.cpp +++ b/src/bun.js/bindings/CommonJSModuleRecord.cpp @@ -1098,7 +1098,7 @@ bool JSCommonJSModule::evaluate( bool isBuiltIn) { auto& vm = globalObject->vm(); - auto sourceProvider = Zig::SourceProvider::create(jsCast(globalObject), source, JSC::SourceProviderSourceType::Program, isBuiltIn); + auto sourceProvider = Zig::SourceProvider::create(jsCast(globalObject), source, isBuiltIn); this->ignoreESModuleAnnotation = source.tag == ResolvedSourceTagPackageJSONTypeModule; if (this->hasEvaluated) return true; @@ -1152,7 +1152,7 @@ std::optional createCommonJSModule( dirname = jsEmptyString(vm); } - auto sourceProvider = Zig::SourceProvider::create(jsCast(globalObject), source, JSC::SourceProviderSourceType::Program, isBuiltIn); + auto sourceProvider = Zig::SourceProvider::create(jsCast(globalObject), source, isBuiltIn); sourceOrigin = sourceProvider->sourceOrigin(); moduleObject = JSCommonJSModule::create( vm, diff --git a/src/bun.js/bindings/ModuleLoader.cpp b/src/bun.js/bindings/ModuleLoader.cpp index 7a939b8385..bb116b0662 100644 --- a/src/bun.js/bindings/ModuleLoader.cpp +++ b/src/bun.js/bindings/ModuleLoader.cpp @@ -745,9 +745,7 @@ static JSValue fetchESMSourceCode( auto tag = res->result.value.tag; switch (tag) { case SyntheticModuleType::ESM: { - JSC::SourceProviderSourceType sourceType = JSC::SourceProviderSourceType::BunTranspiledModule; - if (res->result.value.module_info == nullptr) sourceType = JSC::SourceProviderSourceType::Module; - auto&& provider = Zig::SourceProvider::create(globalObject, res->result.value, sourceType, true); + auto&& provider = Zig::SourceProvider::create(globalObject, res->result.value, true); return rejectOrResolve(JSSourceCode::create(vm, JSC::SourceCode(provider))); } @@ -766,7 +764,7 @@ static JSValue fetchESMSourceCode( auto source = JSC::SourceCode(JSC::SyntheticSourceProvider::create(generateInternalModuleSourceCode(globalObject, static_cast(tag & mask)), JSC::SourceOrigin(URL(makeString("builtins://"_s, moduleKey))), moduleKey)); return rejectOrResolve(JSSourceCode::create(vm, WTFMove(source))); } else { - auto&& provider = Zig::SourceProvider::create(globalObject, res->result.value, JSC::SourceProviderSourceType::BunTranspiledModule, true); + auto&& provider = Zig::SourceProvider::create(globalObject, res->result.value, true); return rejectOrResolve(JSC::JSSourceCode::create(vm, JSC::SourceCode(provider))); } } diff --git a/src/bun.js/bindings/ZigSourceProvider.cpp b/src/bun.js/bindings/ZigSourceProvider.cpp index 53c1236bb8..660de603f5 100644 --- a/src/bun.js/bindings/ZigSourceProvider.cpp +++ b/src/bun.js/bindings/ZigSourceProvider.cpp @@ -75,9 +75,17 @@ extern "C" void Bun__removeSourceProviderSourceMap(void* bun_vm, SourceProvider* Ref SourceProvider::create( Zig::GlobalObject* globalObject, ResolvedSource& resolvedSource, - JSC::SourceProviderSourceType sourceType, bool isBuiltin) { + + JSC::SourceProviderSourceType sourceType = JSC::SourceProviderSourceType::BunTranspiledModule; + if (resolvedSource.isCommonJSModule) { + ASSERT(resolvedSource.module_info == nullptr, "isCommonJSModule should not have module_info"); + sourceType = JSC::SourceProviderSourceType::Program; + } else if (resolvedSource.module_info == nullptr) { + sourceType = JSC::SourceProviderSourceType::Module; + } + auto string = resolvedSource.source_code.toWTFString(BunString::ZeroCopy); auto sourceURLString = resolvedSource.source_url.toWTFString(BunString::ZeroCopy); diff --git a/src/bun.js/bindings/ZigSourceProvider.h b/src/bun.js/bindings/ZigSourceProvider.h index 53a8e1ea99..73e0fbb88c 100644 --- a/src/bun.js/bindings/ZigSourceProvider.h +++ b/src/bun.js/bindings/ZigSourceProvider.h @@ -39,7 +39,6 @@ public: static Ref create( Zig::GlobalObject*, ResolvedSource& resolvedSource, - JSC::SourceProviderSourceType sourceType = JSC::SourceProviderSourceType::BunTranspiledModule, bool isBuiltIn = false); ~SourceProvider(); unsigned hash() const override; diff --git a/src/bun.js/bindings/bindings.cpp b/src/bun.js/bindings/bindings.cpp index 0215f0e523..7df8634f02 100644 --- a/src/bun.js/bindings/bindings.cpp +++ b/src/bun.js/bindings/bindings.cpp @@ -2695,7 +2695,7 @@ JSC__JSValue JSC__JSModuleLoader__evaluate(JSC__JSGlobalObject* globalObject, co JSC::SourceCode sourceCode = JSC::makeSource( src, JSC::SourceOrigin { origin }, JSC::SourceTaintedOrigin::Untainted, origin.fileSystemPath(), - WTF::TextPosition(), JSC::SourceProviderSourceType::BunTranspiledModule); + WTF::TextPosition(), JSC::SourceProviderSourceType::Module); globalObject->moduleLoader()->provideFetch(globalObject, jsString(vm, origin.fileSystemPath()), WTFMove(sourceCode)); auto* promise = JSC::importModule(globalObject, JSC::Identifier::fromString(vm, origin.fileSystemPath()), JSValue(jsString(vm, referrer.fileSystemPath())), JSValue(), JSValue()); diff --git a/src/bun.js/bindings/exports.zig b/src/bun.js/bindings/exports.zig index 9a4c652d68..615a7d008b 100644 --- a/src/bun.js/bindings/exports.zig +++ b/src/bun.js/bindings/exports.zig @@ -222,11 +222,9 @@ pub const ResolvedSource = extern struct { /// - for esm: null means to use jsc's regular parsing step. more info: https://github.com/oven-sh/bun/pull/15758 /// - for cjs: must be null module_info: ?*@import("../../analyze_transpiled_module.zig").ModuleInfoDeserialized, - __is_probably_real: u32 = 0xABEF, pub const unfilled = ResolvedSource{ .module_info = null, - .__is_probably_real = 0xACDE, }; pub const Tag = @import("ResolvedSourceTag").ResolvedSourceTag; diff --git a/src/bun.js/bindings/headers-handwritten.h b/src/bun.js/bindings/headers-handwritten.h index 7938abb2eb..6cf02c5c06 100644 --- a/src/bun.js/bindings/headers-handwritten.h +++ b/src/bun.js/bindings/headers-handwritten.h @@ -109,7 +109,6 @@ typedef struct ResolvedSource { uint8_t* bytecode_cache; size_t bytecode_cache_size; bun_ModuleInfoDeserialized* module_info; - uint32_t __is_probably_real; } ResolvedSource; static const uint32_t ResolvedSourceTagPackageJSONTypeModule = 1; typedef union ErrorableResolvedSourceResult { diff --git a/src/bun.js/bindings/napi.cpp b/src/bun.js/bindings/napi.cpp index a896327506..c32a3ef234 100644 --- a/src/bun.js/bindings/napi.cpp +++ b/src/bun.js/bindings/napi.cpp @@ -110,7 +110,7 @@ JSC::SourceCode generateSourceCode(WTF::String keyString, JSC::VM& vm, JSC::JSOb sourceCodeBuilder.append(";\n"_s); } globalObject->putDirect(vm, ident, object, JSC::PropertyAttribute::DontDelete | JSC::PropertyAttribute::DontEnum); - return JSC::makeSource(sourceCodeBuilder.toString(), JSC::SourceOrigin(), JSC::SourceTaintedOrigin::Untainted, keyString, WTF::TextPosition(), JSC::SourceProviderSourceType::BunTranspiledModule); + return JSC::makeSource(sourceCodeBuilder.toString(), JSC::SourceOrigin(), JSC::SourceTaintedOrigin::Untainted, keyString, WTF::TextPosition(), JSC::SourceProviderSourceType::Module); } } diff --git a/src/bun.js/module_loader.zig b/src/bun.js/module_loader.zig index 81ded3ad03..59c4fe0a54 100644 --- a/src/bun.js/module_loader.zig +++ b/src/bun.js/module_loader.zig @@ -560,6 +560,9 @@ pub const RuntimeTranspilerStore = struct { var module_info: ?*analyze_transpiled_module.ModuleInfoDeserialized = null; if (entry.esm_record.len > 0) { + if (entry.metadata.module_type == .cjs) { + @panic("TranspilerCache contained cjs module with module info"); + } const module_info_data = bun.default_allocator.dupe(u8, entry.esm_record) catch bun.outOfMemory(); // TODO: defer bun.default_allocator.free(module_info_data); const module_info_deserialized = bun.default_allocator.create(analyze_transpiled_module.ModuleInfoDeserialized) catch bun.outOfMemory(); @@ -659,7 +662,8 @@ pub const RuntimeTranspilerStore = struct { var printer = source_code_printer.?.*; printer.ctx.reset(); - const module_info = ModuleInfo.create(bun.default_allocator) catch bun.outOfMemory(); + const is_commonjs_module = parse_result.ast.has_commonjs_export_names or parse_result.ast.exports_kind == .cjs; + const module_info: ?*ModuleInfo = if (is_commonjs_module) null else ModuleInfo.create(bun.default_allocator) catch bun.outOfMemory(); // defer module_info.destroy(); // TODO: do not leak module_info { @@ -711,9 +715,9 @@ pub const RuntimeTranspilerStore = struct { .source_code = source_code, .specifier = duped, .source_url = duped.createIfDifferent(path.text), - .is_commonjs_module = parse_result.ast.has_commonjs_export_names or parse_result.ast.exports_kind == .cjs, + .is_commonjs_module = is_commonjs_module, .hash = 0, - .module_info = module_info.asDeserialized(), + .module_info = if (module_info) |mi| mi.asDeserialized() else null, }; } }; @@ -1432,7 +1436,8 @@ pub const ModuleLoader = struct { var printer = VirtualMachine.source_code_printer.?.*; printer.ctx.reset(); - const module_info = ModuleInfo.create(bun.default_allocator) catch bun.outOfMemory(); + const is_commonjs_module = parse_result.ast.has_commonjs_export_names or parse_result.ast.exports_kind == .cjs; + const module_info: ?*ModuleInfo = if (is_commonjs_module) null else ModuleInfo.create(bun.default_allocator) catch bun.outOfMemory(); // defer module_info.destroy(); // TODO: do not leak module_info { @@ -1469,7 +1474,7 @@ pub const ModuleLoader = struct { } } - resolved_source.is_commonjs_module = parse_result.ast.has_commonjs_export_names or parse_result.ast.exports_kind == .cjs; + resolved_source.is_commonjs_module = is_commonjs_module; return resolved_source; } @@ -1479,10 +1484,10 @@ pub const ModuleLoader = struct { .source_code = bun.String.createLatin1(printer.ctx.getWritten()), .specifier = String.init(specifier), .source_url = String.init(path.text), - .is_commonjs_module = parse_result.ast.has_commonjs_export_names or parse_result.ast.exports_kind == .cjs, + .is_commonjs_module = is_commonjs_module, .hash = 0, - .module_info = module_info.asDeserialized(), + .module_info = if (module_info) |mi| mi.asDeserialized() else null, }; } @@ -1836,6 +1841,9 @@ pub const ModuleLoader = struct { var module_info: ?*analyze_transpiled_module.ModuleInfoDeserialized = null; if (entry.esm_record.len > 0) { + if (entry.metadata.module_type == .cjs) { + @panic("TranspilerCache contained cjs module with module info"); + } const module_info_data = bun.default_allocator.dupe(u8, entry.esm_record) catch bun.outOfMemory(); // TODO: defer bun.default_allocator.free(module_info_data); const module_info_deserialized = bun.default_allocator.create(analyze_transpiled_module.ModuleInfoDeserialized) catch bun.outOfMemory(); @@ -1936,7 +1944,8 @@ pub const ModuleLoader = struct { printer.ctx.reset(); defer source_code_printer.* = printer; - const module_info = ModuleInfo.create(bun.default_allocator) catch bun.outOfMemory(); + const is_commonjs_module = parse_result.ast.has_commonjs_export_names or parse_result.ast.exports_kind == .cjs; + const module_info: ?*ModuleInfo = if (is_commonjs_module) null else ModuleInfo.create(bun.default_allocator) catch bun.outOfMemory(); // defer module_info.destroy(); // TODO: do not leak module_info _ = brk: { @@ -2001,10 +2010,10 @@ pub const ModuleLoader = struct { }, .specifier = input_specifier, .source_url = input_specifier.createIfDifferent(path.text), - .is_commonjs_module = parse_result.ast.has_commonjs_export_names or parse_result.ast.exports_kind == .cjs, + .is_commonjs_module = is_commonjs_module, .hash = 0, .tag = tag, - .module_info = module_info.asDeserialized(), + .module_info = if (module_info) |mi| mi.asDeserialized() else null, }; }, // provideFetch() should be called