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

Cleanup js::core::Context, move functionality to ExtensionInterfaces #6163

Merged
merged 35 commits into from
May 17, 2024

Conversation

eddyashton
Copy link
Member

@eddyashton eddyashton commented May 2, 2024

Goal is for js::core::Context to be a pure interpreter (ie - we don't insert anything that QuickJS doesn't), and a vaguely readable interface. This PR works towards the latter by shuffling methods around, removing a few unused/unnecessary overloads, and the former by introducing an Extension pattern rather than populate_global_XXX methods on the Context.

A goal of this extension pattern is the removal of the messy globals object on Context - previously lots of inserted calls relied on the globals.tx object inserted to support the ccf.kv functions. Now they explicitly use the state in their own extension, including a Tx* where necessary. This should also simplify cleanup, as its a case of deleting extensions rather than invalidating global values.

@eddyashton eddyashton marked this pull request as ready for review May 16, 2024 15:36
@eddyashton eddyashton requested a review from a team as a code owner May 16, 2024 15:36
@eddyashton
Copy link
Member Author

eddyashton commented May 16, 2024

TODO before merging:

  • Rename IExtension
  • Rename the extensions for less Ccf repetition
  • Manually verify what we expose in each execution context hasn't changed, see if that can be automated. (EDIT: Manually verified that we insert exactly the same globals, but with a bunch of hacks and manually string mangling. Will punt on automating this for now.

@eddyashton eddyashton changed the title [WIP] Cleanup js::core::Context, move to an extension model Cleanup js::core::Context, move functionality to ExtensionInterfaces May 17, 2024
@achamayou achamayou enabled auto-merge (squash) May 17, 2024 14:23
@achamayou achamayou merged commit b10483a into microsoft:main May 17, 2024
25 of 31 checks passed
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.

None yet

2 participants