[2026-10-07] eagle: work/projects/vscode-migration/tech-design-spm-browser-package.md

This commit is contained in:
Alexey Martemyanov committed 2026-10-07 13:07:21 +06:00
1 parent 8224dbb53e
commit f35e128788
1 file changed
+18 -18
@@ -132,27 +132,27 @@ DMG / App Store app targets (module DuckDuckGoApp):
Already given up in earlier, merged steps of this project:
| Lost | Why | Mitigation |
|---|---|---|
| Per-target compile-time flags (`SPARKLE`, `APPSTORE`, `ALPHA`, `SANDBOX_TEST_TOOL`, `NETWORK_EXTENSION` in app files) | A package target has one set of conditions for all products that link it | Runtime checks (`StandardApplicationBuildType`, bundle/feature checks); DMG/App Store differences live in injected services |
| Storyboards and xibs | Interface Builder module binding (`customModuleProvider="target"`) breaks when code leaves the app module | Views built in code / SwiftUI |
| Generated asset symbols (`NSImage.foo`, `NSColor.foo`) | Xcode doesn't generate these symbol extensions for packages (Xcode 26 and 27) | `NSImage(resource: .foo)` / `NSColor(resource:)`, type-checked |
| Sandbox test tool | A separate target that compiled app sources | `AppStoreSandboxUITests` on CI (#6980) |
| Lost | Why | Mitigation |
| --------------------------------------------------------------------------------------------------------------------- | --------------------------------------------------------------------------------------------------------- | --------------------------------------------------------------------------------------------------------------------------- |
| Per-target compile-time flags (`SPARKLE`, `APPSTORE`, `ALPHA`, `SANDBOX_TEST_TOOL`, `NETWORK_EXTENSION` in app files) | A package target has one set of conditions for all products that link it | Runtime checks (`StandardApplicationBuildType`, bundle/feature checks); DMG/App Store differences live in injected services |
| Storyboards and xibs | Interface Builder module binding (`customModuleProvider="target"`) breaks when code leaves the app module | Views built in code / SwiftUI |
| Generated asset symbols (`NSImage.foo`, `NSColor.foo`) | Xcode doesn't generate these symbol extensions for packages (Xcode 26 and 27) | `NSImage(resource: .foo)` / `NSColor(resource:)`, type-checked |
| Sandbox test tool | A separate target that compiled app sources | `AppStoreSandboxUITests` on CI (#6980) |
New with this step:
| Lost / changed | Impact | Mitigation |
|---|---|---|
| **Resources must be declared in `Package.swift`** | A new resource type or folder that isn't listed is not bundled. SwiftPM only warns: "found N file(s) which are unhandled" | `InputFilesChecker` verifies package vs Xcode target ownership; **treat SwiftPM's unhandled-files warning as a CI error**; recommend folder-level `.process("Feature/Resources")` entries so new files in existing folders are picked up |
| **`Bundle.main` no longer finds browser resources** | `NSImage(named:)`, `Bundle.main.url(forResource:)`, `LottieAnimation.asset(_, bundle: .main)` silently return nil or crash at runtime (3 such regressions were caught and fixed on the branch) | **SwiftLint rules** in `macOS/.swiftlint.yml`: forbid `NSImage(named:)`/`NSColor(named:)` and resource lookups on `Bundle.main` in `macOS/DuckDuckGo/` (use `NSImage(resource:)` or `Bundle.module`); identity uses (`bundleIdentifier`, `bundleURL`, app group) stay allowed. Plus a unit test that loads every bundled Lottie/data asset (added) |
| **Localization needs the calling module's bundle** | SwiftUI `Text("literal")` and `String(localized:)` default to `Bundle.main`; a missing `bundle:` shows the English key at runtime with no build error | Module-level `NSLocalizedString` shim with `#bundle`; **SwiftLint rule**: `Text("…")` / `String(localized:)` / `LocalizedStringKey` in the package must pass `bundle: #bundle`. The translation checks were updated to follow the catalog's move |
| **No cross-linking of a source file into several targets** | Helper targets (VPN, PIR agents, extensions) can't simply add an app file to their Sources phase any more | Code shared with helpers goes into a shared local package (`AppKitExtensions`, `DataBrokerProtection-macOS`, `NetworkProtectionMac/VPNPixels`, …). The few remaining files are still referenced by path and tracked in `InputFilesChecker`'s temporary list (Notes [4]) |
| **Custom configurations don't reach the package** | Packages know only debug/release. Review and CI map by name; per-configuration xcconfig Swift settings (flags, warnings) don't apply to package targets | `SPM_FORCE_DEBUG` for CI; `REVIEW` replaced by a runtime check; Swift settings (language mode, warnings) set in `Package.swift` to match the app's xcconfig |
| **SwiftPM manifest cache ignores environment variables** | Locally switching between `SPM_FORCE_DEBUG=1` and normal builds can reuse a stale evaluated manifest | Documented; CI runners are clean. Same pre-existing risk as `AppKitExtensions` |
| **Two `Package.resolved` files** | `macOS/DuckDuckGo/Package.resolved` (for standalone SwiftPM builds) can drift from the project's | CI check that the pinned versions match, or generate the file from the project's |
| **Xcode 27 required** | Package asset catalogs, Core Data codegen and string catalogs | The project already targets Xcode 27 (Xcode 26 support is dropped) |
| **App module renamed to `DuckDuckGoApp`** | Anything that referred to the app module by name (`Bundle(for:)` from app code, `NSClassFromString("DuckDuckGo_Privacy_Browser.X")`) | All code is in the package, which keeps the old name; the app module holds only `main.swift` |
| **Build time not yet measured** | One big package target vs the app target: clean and incremental times may change either way | Benchmark clean and incremental builds (Debug, Release) before merging; see Testing |
| Lost / changed | Impact | Mitigation |
| ---------------------------------------------------------- | ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- |
| **Resources must be declared in `Package.swift`** | A new resource type or folder that isn't listed is not bundled. SwiftPM only warns: "found N file(s) which are unhandled" | `InputFilesChecker` verifies package vs Xcode target ownership; **treat SwiftPM's unhandled-files warning as a CI error**; recommend folder-level `.process("Feature/Resources")` entries so new files in existing folders are picked up |
| **`Bundle.main` no longer finds browser resources** | `NSImage(named:)`, `Bundle.main.url(forResource:)`, `LottieAnimation.asset(_, bundle: .main)` silently return nil or crash at runtime (3 such regressions were caught and fixed on the branch) | **SwiftLint rules** in `macOS/.swiftlint.yml`: forbid `NSImage(named:)`/`NSColor(named:)` and resource lookups on `Bundle.main` in `macOS/DuckDuckGo/` (use `NSImage(resource:)` or `Bundle.module`); identity uses (`bundleIdentifier`, `bundleURL`, app group) stay allowed. Plus a unit test that loads every bundled Lottie/data asset (added) |
| **Localization needs the calling module's bundle** | SwiftUI `Text("literal")` and `String(localized:)` default to `Bundle.main`; a missing `bundle:` shows the English key at runtime with no build error | Module-level `NSLocalizedString` shim with `#bundle`; **SwiftLint rule**: `Text("…")` / `String(localized:)` / `LocalizedStringKey` in the package must pass `bundle: #bundle`. The translation checks were updated to follow the catalog's move |
| **No cross-linking of a source file into several targets** | Helper targets (VPN, PIR agents, extensions) can't simply add an app file to their Sources phase any more | Code shared with helpers goes into a shared local package (`AppKitExtensions`, `DataBrokerProtection-macOS`, `NetworkProtectionMac/VPNPixels`, …). The few remaining files are still referenced by path and tracked in `InputFilesChecker`'s temporary list (Notes [4]) |
| **Custom configurations don't reach the package** | Packages know only debug/release. Review and CI map by name; per-configuration xcconfig Swift settings (flags, warnings) don't apply to package targets | `SPM_FORCE_DEBUG` for CI; `REVIEW` replaced by a runtime check; Swift settings (language mode, warnings) set in `Package.swift` to match the app's xcconfig |
| **SwiftPM manifest cache ignores environment variables** | Locally switching between `SPM_FORCE_DEBUG=1` and normal builds can reuse a stale evaluated manifest | Documented; CI runners are clean. Same pre-existing risk as `AppKitExtensions` |
| **Two `Package.resolved` files** | `macOS/DuckDuckGo/Package.resolved` (for standalone SwiftPM builds) can drift from the project's | CI check that the pinned versions match, or generate the file from the project's |
| **Xcode 27 required** | Package asset catalogs, Core Data codegen and string catalogs | The project already targets Xcode 27 (Xcode 26 support is dropped) |
| **App module renamed to `DuckDuckGoApp`** | Anything that referred to the app module by name (`Bundle(for:)` from app code, `NSClassFromString("DuckDuckGo_Privacy_Browser.X")`) | All code is in the package, which keeps the old name; the app module holds only `main.swift` |
| **Build time not yet measured** | One big package target vs the app target: clean and incremental times may change either way | Benchmark clean and incremental builds (Debug, Release) before merging; see Testing |
**What else could be easy to miss:**
- **App Store and notarization:** the SwiftPM resource bundle (`DuckDuckGo_DuckDuckGo_Privacy_Browser.bundle`) is now nested in `Contents/Resources`. It must pass App Store validation and notarization, and be signed with the app. Verify with a TestFlight/App Store Connect validation build.