[TIMOB-20557] Allow modules to use third party dynamic libraries #9346
Conversation
janvennemann
force-pushed
the
TIMOB-20557
branch
from
834fc56
to
20f22a0
Compare
iphone/cli/hooks/frameworks.js
Outdated
| * @param {frameworkInfo} frameworkInfo Framework metadata | ||
| * @param {string} fileRefUuid Uuid of the frameworks file reference inside the Xcode project | ||
| */ | ||
| addEmbeddFrameworkBuildPhase(frameworkInfo, fileRefUuid) { |
There was a problem hiding this comment.
Looks like you either have an extra d or missing an ed.
iphone/cli/hooks/frameworks.js
Outdated
| * @param {Object} cli CLI instance | ||
| * @param {Object} config Project configuration | ||
| * @param {Object} logger Logger instance | ||
| */ |
There was a problem hiding this comment.
All class methods, including constructors, should have an @access jsdoc after all other jsdoc tokens. Possible values are @access public or @access private.
iphone/cli/hooks/frameworks.js
Outdated
| */ | ||
| detectBinaryTypeAndArchitectures(binaryPathAndFilename) { | ||
| return new Promise((resolve, reject) => { | ||
| exec('file -b "' + binaryPathAndFilename + '"', (error, stdout) => { |
There was a problem hiding this comment.
I'm sure this works fine. Generally I prefer spawn() or spawnSync() over exec(). It's more robust and you won't run into problems should binaryPathAndFilename contain a double quote.
iphone/cli/hooks/frameworks.js
Outdated
| return Promise.resolve(); | ||
| } | ||
|
|
||
| let incrementalDirectory = path.join(this._builder.projectDir, 'build', 'incremental'); |
There was a problem hiding this comment.
In general, you should always use const unless you need to use let. This also includes for loops.
iphone/cli/hooks/frameworks.js
Outdated
| /** | ||
| * Finds any .framework directories inside the project and its modules | ||
| * | ||
| * @return {Array} Array of available third-party framework directory paths |
There was a problem hiding this comment.
This function actually returns a Promise.
iphone/cli/hooks/frameworks.js
Outdated
| /** | ||
| * Add the framework as a new file reference to the Xcode project | ||
| * | ||
| * @param {frameworkInfo} frameworkInfo Framework metadata |
There was a problem hiding this comment.
I believe the type should be FrameworkInfo. There are a couple other places below with the same issue.
iphone/cli/hooks/frameworks.js
Outdated
| * Add the framework as a new file reference to the Xcode project | ||
| * | ||
| * @param {frameworkInfo} frameworkInfo Framework metadata | ||
| * @return {string} Uuid of the created file reference |
There was a problem hiding this comment.
All built-in types should be capitalized: String.
iphone/cli/hooks/frameworks.js
Outdated
| this._xobjs.PBXFileReference[fileRefUuid] = { | ||
| isa: 'PBXFileReference', | ||
| lastKnownFileType: 'wrapper.framework', | ||
| name: '"' + frameworkPackageName + '"', |
There was a problem hiding this comment.
Consider using ES6 template strings to make the code more readable:
name: `"${frameworkPackageName}"`,
iphone/cli/hooks/frameworks.js
Outdated
| * @return {Promise} | ||
| */ | ||
| detectFrameworks() { | ||
| return this.findFrameworkPaths().then((frameworkPaths) => { |
There was a problem hiding this comment.
Generally, I recommend putting the .then() on the next line. This makes it a little more readable, especially if there's a .catch().
iphone/cli/hooks/frameworks.js
Outdated
| } | ||
|
|
||
| resolve({ | ||
| type: type, |
There was a problem hiding this comment.
You can use shorthand properties here.
resolve({ type, architectures });
|
Awesome input man, thanks! Addressed everything and updated the PR. |
JIRA: https://jira.appcelerator.org/browse/TIMOB-20557
Optional Description:
This adds a fully automated framework configuration including support for dynamic frameworks
platform/ios/strip-frameworks.shin project.-frameworklinker flags). Either by restricting the automated configuration of frameworks to a subdirectory, e.g.Frameworksso the new mechanism does not interfere with existing modules, or by intelligently skipping the automated Xcode project integration for frameworks that have a .xcconfig (checking only for the-frameworklinker flag should probably be sufficient). EDIT: This seems to be a non issue. Xcode has no problems with multiple-frameworkflags and the framework search path already has a dupe check.