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

feat: allow embedding into Swift host projects #231

Merged
merged 11 commits into from
Jul 1, 2024

Conversation

tdermendjiev
Copy link
Contributor

This PR includes methods required for running NS apps and js strings from a native app using the NS runtime:

  • (void)runScriptString: (NSString*) script runLoop: (BOOL) runLoop;
  • (void)restartWithConfig:(Config*)config;
  • (void)shutdownRuntime;

A default metadata binary is added to the framework product. This contains metadata about all the Apple frameworks.
TBD: Create a separate build target for the framework used for embedding.

@cla-bot cla-bot bot added the cla: yes label Oct 19, 2023
@NathanWalker NathanWalker changed the base branch from main to embed October 20, 2023 16:23
@edusperoni edusperoni changed the base branch from embed to main April 2, 2024 16:27
Comment on lines 61 to 67
Isolate* isolate = runtime_->GetIsolate();
{
v8::Locker l(isolate);
v8::Isolate::Scope isolate_scope(isolate);
v8::HandleScope handle_scope(isolate);

isolate->Dispose();
Copy link
Collaborator

Choose a reason for hiding this comment

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

this likely shouldn't be done here (the runtime destructor is responsible for deleting this isolate)

@@ -59,25 +109,20 @@ - (instancetype)initWithConfig:(Config*)config {
}
}

return self;
Copy link
Collaborator

@edusperoni edusperoni Apr 2, 2024

Choose a reason for hiding this comment

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

shouldn't this still return self?

Comment on lines 115 to 117
if (self = [super init]) {
[self initializeWithConfig:config];
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

init will be called twice if this method is called (one here and another in initializeWithConfig)

@tdermendjiev tdermendjiev changed the title WIP: Implement embedding APIs Implement embedding APIs Apr 11, 2024
@NathanWalker NathanWalker changed the base branch from main to alpha May 28, 2024 17:58
@NathanWalker NathanWalker changed the title Implement embedding APIs feat: allow embedding into Swift host projects May 28, 2024
@NathanWalker NathanWalker changed the base branch from alpha to embed May 28, 2024 17:59
Copy link
Contributor

Choose a reason for hiding this comment

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

Are all these changes necessary in this pbxproj file @tdermendjiev ?

@edusperoni
Copy link
Collaborator

can we just confirm this all works with existing projects? I'm not sure about the implications on the changes to modules in the pbxproj. But once that's confirmed I believe this can be merged

@edusperoni edusperoni changed the base branch from embed to main June 26, 2024 14:46
@@ -392,6 +395,7 @@
dstSubfolderSpec = 10;
files = (
C20525802577D6F900C12A5C /* NativeScript.framework in Embed Frameworks */,
2B5088A82BBEC1BC00F6EB68 /* TNSWidgets.xcframework in Embed Frameworks */,
Copy link
Collaborator

Choose a reason for hiding this comment

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

is this addition necessary?

Copy link
Collaborator

Choose a reason for hiding this comment

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

how do we generate this again when the metadata is updated? Shouldn't this just be part of the build process?

@NathanWalker NathanWalker merged commit 7ab180a into NativeScript:main Jul 1, 2024
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants