fix: make process.mainModule writable
#18730
Conversation
|
Updated 5:27 PM PT - Apr 9th, 2025
❌ @Jarred-Sumner, your commit 7633980 has 1 failures in
🧪 try this PR locally: bunx bun-pr 18730 |
DonIsaac
requested review from
a team and
cirospaciari
and removed request for
a team
There was a problem hiding this comment.
The private variable is unnecessary, and calling BunObject__getter_wrap_main is non-trivial overhead
Instead, we can add a LazyProperty onto Zig::GlobalObject and make Bun.main access that LazyProperty and the process.mainModule getter & setter can get/set that LazyProperty
| } | ||
|
|
||
| // NOTE: we cannot assume `main` is ascii. | ||
| var main_module = bun.String.init(bun_vm.main); |
There was a problem hiding this comment.
| var main_module = bun.String.init(bun_vm.main); | |
| return bun.String.createUTF8ForJS(bun_vm.main); |
| @@ -3335,6 +3336,13 @@ void GlobalObject::finishCreation(VM& vm) | |||
| InternalModuleRegistry::createStructure(init.vm, init.owner))); | |||
| }); | |||
|
|
|||
| m_mainModule.initLater( | |||
There was a problem hiding this comment.
Where is this visited?
| return JSValue::encode(jsUndefined()); | ||
| } | ||
| auto prop = PropertyName(JSC::Identifier::fromString(vm, WTFMove(mm->getString(global)))); | ||
| auto mainModule = requireCache->getIfPropertyExists(global, prop); |
There was a problem hiding this comment.
Why is this accessing the require object? Why isn't this a getter/setter for the same main module LazyProperty defined on the global object? The require.main and the process.mainModule implementations should both access the same underlying WriteBarrier
What does this PR do?
Add a setter for
process.mainModule. This snippet works in node but not in bun, causing some older test runners to breakHow did you verify your code works?
I've added tests