-
-
Notifications
You must be signed in to change notification settings - Fork 33
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
feat: allow embedding into Swift host projects #231
Conversation
NativeScript/NativeScript.mm
Outdated
Isolate* isolate = runtime_->GetIsolate(); | ||
{ | ||
v8::Locker l(isolate); | ||
v8::Isolate::Scope isolate_scope(isolate); | ||
v8::HandleScope handle_scope(isolate); | ||
|
||
isolate->Dispose(); |
There was a problem hiding this comment.
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; |
There was a problem hiding this comment.
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?
NativeScript/NativeScript.mm
Outdated
if (self = [super init]) { | ||
[self initializeWithConfig:config]; | ||
} |
There was a problem hiding this comment.
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)
There was a problem hiding this comment.
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 ?
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 |
v8ios.xcodeproj/project.pbxproj
Outdated
@@ -392,6 +395,7 @@ | |||
dstSubfolderSpec = 10; | |||
files = ( | |||
C20525802577D6F900C12A5C /* NativeScript.framework in Embed Frameworks */, | |||
2B5088A82BBEC1BC00F6EB68 /* TNSWidgets.xcframework in Embed Frameworks */, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is this addition necessary?
NativeScript/metadata-arm64.bin
Outdated
There was a problem hiding this comment.
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?
This PR includes methods required for running NS apps and js strings from a native app using the NS runtime:
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.