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

Support builtin modules #32

Merged
merged 3 commits into from
Apr 25, 2024

Conversation

guybedford
Copy link
Contributor

This adds support for defining builtin modules from JS objects, which are then lazily turned into synthetic modules when imported, based on enumerating the own keys of the object and making those the exports of the synthetic module.

Useful for supporting custom internal APIs without needing to define globals.

@guybedford
Copy link
Contributor Author

While we still don't have tests of building builtins, this is a little tricky to get coverage on, but I can confirm that it is working in my work-in-progress branch to support Fastly builtins.

@guybedford guybedford force-pushed the define-builtins branch 2 times, most recently from aaf91d2 to d0acb1d Compare April 19, 2024 17:43
*
* Once loaded, the instance is cached and reused as a singleton.
*/
bool define_builtin_import(const char* id, HandleValue builtin);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

An example usage here might look like:

define_builtin_import("fastly:env", envObject);

Copy link
Member

@tschneidereit tschneidereit left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is great, thank you so much!

include/extension-api.h Outdated Show resolved Hide resolved
runtime/engine.cpp Outdated Show resolved Hide resolved
runtime/script_loader.cpp Outdated Show resolved Hide resolved
runtime/script_loader.cpp Show resolved Hide resolved
Comment on lines +162 to +166
if (firstValue) {
firstValue = false;
} else {
code += ", ";
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not at all important, but I think it'd be fine to append the , unconditionally, and then just remove the leading space before the closing }.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah agreed, although I still prefer the proper formatting I must admit!

runtime/script_loader.cpp Outdated Show resolved Hide resolved
@guybedford guybedford merged commit c26001c into bytecodealliance:main Apr 25, 2024
1 check passed
@guybedford guybedford deleted the define-builtins branch April 25, 2024 17:30
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