-
Notifications
You must be signed in to change notification settings - Fork 1.2k
fix(cli): generate resource entries in Package.swift #8455
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -128,6 +128,19 @@ function buildBinaryTargetEntries(p: Plugin, frameworks: any[]): { binaryTargets | |||||||||||||||||||||||||||||||||||||||
| return { binaryTargetsText, binaryDepsText }; | ||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||
| function buildResourcesText(resources: any[]) { | ||||||||||||||||||||||||||||||||||||||||
| const resourceEntry = []; | ||||||||||||||||||||||||||||||||||||||||
| for (const resource of resources) { | ||||||||||||||||||||||||||||||||||||||||
| resourceEntry.push(`.copy("resources/${resource.$.src.split('/').pop()}")`); | ||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||
| return resources.length > 0 | ||||||||||||||||||||||||||||||||||||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. A little confusing what's going on here. Could we split this up without the ternary? I thought it was returning bool, and then thought there was dead code, before I noticed what was up.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is in line with other string generations in the codebase: capacitor/cli/src/ios/update.ts Line 204 in 1f7e33f
capacitor/cli/src/ios/update.ts Lines 189 to 192 in 1f7e33f
capacitor/cli/src/ios/update.ts Lines 206 to 211 in 1f7e33f
Lines 137 to 144 in 1f7e33f
|
||||||||||||||||||||||||||||||||||||||||
| ? `, | ||||||||||||||||||||||||||||||||||||||||
| resources: [ | ||||||||||||||||||||||||||||||||||||||||
| ${resourceEntry.join(',\n ')} | ||||||||||||||||||||||||||||||||||||||||
| ]` | ||||||||||||||||||||||||||||||||||||||||
| : ''; | ||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||
| function buildCSettingsText(p: Plugin, sourceFiles: any[]): string { | ||||||||||||||||||||||||||||||||||||||||
| const pluginId = p.id; | ||||||||||||||||||||||||||||||||||||||||
| const allFlags = new Set<string>(); | ||||||||||||||||||||||||||||||||||||||||
|
|
@@ -190,8 +203,9 @@ async function writeGeneratedPackageSwift(p: Plugin, config: Config, iosPlatform | |||||||||||||||||||||||||||||||||||||||
| ? `, | ||||||||||||||||||||||||||||||||||||||||
| publicHeadersPath: "."` | ||||||||||||||||||||||||||||||||||||||||
| : ''; | ||||||||||||||||||||||||||||||||||||||||
| const resources = getPlatformElement(p, platform, 'resource-file'); | ||||||||||||||||||||||||||||||||||||||||
| const sourceFiles = getPlatformElement(p, platform, 'source-file'); | ||||||||||||||||||||||||||||||||||||||||
| if (sourceFiles.length === 0 && headerFiles.length === 0) { | ||||||||||||||||||||||||||||||||||||||||
| if (sourceFiles.length === 0 && headerFiles.length === 0 && resources.length === 0) { | ||||||||||||||||||||||||||||||||||||||||
| return; | ||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||
| const frameworks = getPlatformElement(p, platform, 'framework'); | ||||||||||||||||||||||||||||||||||||||||
|
|
@@ -200,6 +214,7 @@ async function writeGeneratedPackageSwift(p: Plugin, config: Config, iosPlatform | |||||||||||||||||||||||||||||||||||||||
| const systemFrameworks = frameworks.filter((f: any) => !f.$.custom && f.$.src.endsWith('.framework')); | ||||||||||||||||||||||||||||||||||||||||
| const hasWeakFrameworks = systemFrameworks.some((f: any) => f.$.weak === 'true'); | ||||||||||||||||||||||||||||||||||||||||
| const requiredSystemFrameworks = systemFrameworks.filter((f: any) => f.$.weak !== 'true'); | ||||||||||||||||||||||||||||||||||||||||
| const resourcesText = buildResourcesText(resources); | ||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||
| const libraryTypeText = hasWeakFrameworks ? `\n type: .dynamic,` : ''; | ||||||||||||||||||||||||||||||||||||||||
| const linkerSettingsText = | ||||||||||||||||||||||||||||||||||||||||
|
|
@@ -232,7 +247,7 @@ let package = Package( | |||||||||||||||||||||||||||||||||||||||
| dependencies: [ | ||||||||||||||||||||||||||||||||||||||||
| .product(name: "Cordova", package: "capacitor-swift-pm")${binaryDepsText} | ||||||||||||||||||||||||||||||||||||||||
| ], | ||||||||||||||||||||||||||||||||||||||||
| path: "."${headersText}${cSettingsText}${linkerSettingsText} | ||||||||||||||||||||||||||||||||||||||||
| path: "."${resourcesText}${headersText}${cSettingsText}${linkerSettingsText} | ||||||||||||||||||||||||||||||||||||||||
| )${binaryTargetsText} | ||||||||||||||||||||||||||||||||||||||||
| ] | ||||||||||||||||||||||||||||||||||||||||
| )`; | ||||||||||||||||||||||||||||||||||||||||
|
|
@@ -557,8 +572,16 @@ async function copyPluginsNativeFiles(config: Config, cordovaPlugins: Plugin[]) | |||||||||||||||||||||||||||||||||||||||
| fileContent.includes('[NSBundle bundleForClass:[self class]]') || | ||||||||||||||||||||||||||||||||||||||||
| fileContent.includes('[NSBundle bundleForClass:[CDVCapture class]]') | ||||||||||||||||||||||||||||||||||||||||
| ) { | ||||||||||||||||||||||||||||||||||||||||
| fileContent = fileContent.replace('[NSBundle bundleForClass:[self class]]', '[NSBundle mainBundle]'); | ||||||||||||||||||||||||||||||||||||||||
| fileContent = fileContent.replace('[NSBundle bundleForClass:[CDVCapture class]]', '[NSBundle mainBundle]'); | ||||||||||||||||||||||||||||||||||||||||
| const bundleName = isSPM ? 'SWIFTPM_MODULE_BUNDLE' : '[NSBundle mainBundle]'; | ||||||||||||||||||||||||||||||||||||||||
| fileContent = fileContent.replace('[NSBundle bundleForClass:[self class]]', bundleName); | ||||||||||||||||||||||||||||||||||||||||
| fileContent = fileContent.replace('[NSBundle bundleForClass:[CDVCapture class]]', bundleName); | ||||||||||||||||||||||||||||||||||||||||
| await writeFile(fileDest, fileContent, { encoding: 'utf-8' }); | ||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||
| if (isSPM && fileContent.includes('[NSBundle mainBundle] URLForResource')) { | ||||||||||||||||||||||||||||||||||||||||
| fileContent = fileContent.replace( | ||||||||||||||||||||||||||||||||||||||||
| '[NSBundle mainBundle] URLForResource', | ||||||||||||||||||||||||||||||||||||||||
| 'SWIFTPM_MODULE_BUNDLE URLForResource', | ||||||||||||||||||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||||||||||||||||||
|
ItsChaceD marked this conversation as resolved.
|
||||||||||||||||||||||||||||||||||||||||
| await writeFile(fileDest, fileContent, { encoding: 'utf-8' }); | ||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||
| if (fileContent.includes('[self.webView superview]') || fileContent.includes('self.webView.superview')) { | ||||||||||||||||||||||||||||||||||||||||
|
|
@@ -572,10 +595,8 @@ async function copyPluginsNativeFiles(config: Config, cordovaPlugins: Plugin[]) | |||||||||||||||||||||||||||||||||||||||
| const resourceFiles = getPlatformElement(p, platform, 'resource-file'); | ||||||||||||||||||||||||||||||||||||||||
| for (const resourceFile of resourceFiles) { | ||||||||||||||||||||||||||||||||||||||||
| const fileName = resourceFile.$.src.split('/').pop(); | ||||||||||||||||||||||||||||||||||||||||
| await copy( | ||||||||||||||||||||||||||||||||||||||||
| getFilePath(config, p, resourceFile.$.src), | ||||||||||||||||||||||||||||||||||||||||
| join(config.ios.cordovaPluginsDirAbs, 'resources', fileName), | ||||||||||||||||||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||||||||||||||||||
| const rootResFolder = isSPM ? sourcesFolder : config.ios.cordovaPluginsDirAbs; | ||||||||||||||||||||||||||||||||||||||||
| await copy(getFilePath(config, p, resourceFile.$.src), join(rootResFolder, 'resources', fileName)); | ||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||
| for (const framework of frameworks) { | ||||||||||||||||||||||||||||||||||||||||
| if (framework.$.custom && framework.$.custom === 'true') { | ||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||
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.
Also, not sure if this is a real concern, but
buildResourcesTextuses.copy()for all resource types. For resources like.xcassets,.storyboard, and.xib, SPM typically requires.process()to compile them correctly, otherwise they might not be accessible at runtime. That said, the best way to validate this would probably be with a real plugin that actually uses these file types, since the dummy plugin I created for testing had XIB/xcassets files that weren't properly generated by Xcode. If the plugin you tested already included these file types and everything worked fine, then this is probably not a concern!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.
I've only found plugins using
.bundlefile extension and they required.copy.For those file extensions we shouldn't even need to add a resources entry as it's supposed to auto detect them according to the docs, but having them with the copy might conflict, so we will have to revisit if some user reports that they don't work and provide some Cordova plugin to reproduce. We will merge as it is now.
https://developer.apple.com/documentation/xcode/bundling-resources-with-a-swift-package