Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Cross-module calls should execute in the context of the callee #177

Merged
merged 2 commits into from
Oct 31, 2024

Conversation

deregtd
Copy link
Collaborator

@deregtd deregtd commented Oct 30, 2024

The original design was to use the context of the caller in the functions being called, but after we tried to implement a function call this way realized that it was inverting ownership from usual function ownership. The idea was to be able to use helper functions in the context of the caller, but the reality is that you want to pass in any needed context to the exported function, just like in standard programming. This way the function owner can have "private" state/context inside the module that the function exposes a part of, per standard modules in general software engineering.

Copy link

cloudflare-workers-and-pages bot commented Oct 30, 2024

Deploying stencil with  Cloudflare Pages  Cloudflare Pages

Latest commit: 495be80
Status: ✅  Deploy successful!
Preview URL: https://14ca8f12.stencil-1wj.pages.dev
Branch Preview URL: https://ddr-crossmodule.stencil-1wj.pages.dev

View logs

Copy link

codecov bot commented Oct 30, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

📢 Thoughts on this report? Let us know!

@jaredallard
Copy link
Member

Mind filling out the desc for why on this change for the future?

I'll need to look at this one more in-depth, we pass around template in various places to make it easier to know what's going on. The downside there is that I'm very sure that there's something missed in this.

@jaredallard
Copy link
Member

Ok, I checked it out. I forgot that I made sure this would be safe by having the Tpl<Type> structs which did the comixing. Because of this and NewFuncMap this should be safe.

@jaredallard
Copy link
Member

module.Call's function comment needs to be updated to reflect this as well as a doc regen with that.

@deregtd deregtd merged commit a825d4d into main Oct 31, 2024
5 checks passed
@deregtd deregtd deleted the ddr/crossmodule branch October 31, 2024 02:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants