Simplify Networking into a Swift package - Sources only #15737

mokagio · 2025-06-12T07:22:46Z

Description

See https://linear.app/a8c/issue/AINFRA-470.

I've given up on the idea of migrating all frameworks to packages because of a number of test issues I've run into due to shared files and the difference in how SwiftPM handles Bundle.

To avoid the work getting stale, the increasing risk of divergence, and to get feedback on at least some of it ASAP, I've decided to ignore migrating the tests for now and focus on the sources instead.

This first PR moves Networking. Noticeably, it splits it into NetworkingCore and Networking, with the same pattern we used for WooFoundation in #15651.

Steps to reproduce & Testing information

See green CI. Also check out the prototype build.

Screenshots

I have considered if this change warrants user-facing release notes and have added them to RELEASE-NOTES.txt if necessary. — N.A.

dangermattic · 2025-06-12T07:23:19Z

	2 Warnings
⚠️	View files have been modified, but no screenshot or video is included in the pull request. Consider adding some for clarity.
⚠️	This PR is larger than 300 lines of changes. Please consider splitting it into smaller PRs for easier and faster reviews.

Generated by 🚫 Danger

wpmobilebot · 2025-06-12T07:32:20Z

📲 You can test the changes from this Pull Request in WooCommerce iOS Prototype by scanning the QR code below to install the corresponding build.

	App Name	WooCommerce iOS Prototype
	Build Number	`30545`
	Version	`PR #15737`
	Bundle ID	`com.automattic.alpha.woocommerce`
	Commit	`1c2f4e6`
	Installation URL	32bdscg5lj7vo

Automatticians: You can use our internal self-serve MC tool to give yourself access to those builds if needed.

mokagio · 2025-06-12T07:51:46Z

Fakes/Fakes.xcodeproj/project.pbxproj

 		26EEDC9126FE1C7B00D5BA0E /* Yosemite.generated.swift in Sources */ = {isa = PBXBuildFile; fileRef = 26EEDC8F26FE1C7B00D5BA0E /* Yosemite.generated.swift */; };
 		3F2B4AC22DDC30B200E5E49C /* XcodeTarget_Fakes in Frameworks */ = {isa = PBXBuildFile; productRef = 3F2B4AC12DDC30B200E5E49C /* XcodeTarget_Fakes */; };
 		3F37E1232DEEAC7200D8BF2B /* WooFoundationCore.generated.swift in Sources */ = {isa = PBXBuildFile; fileRef = 3F37E1222DEEAC7200D8BF2B /* WooFoundationCore.generated.swift */; };
+		3F38F3742DFA9E0800E0FE41 /* NetworkingCore.generated.swift in Sources */ = {isa = PBXBuildFile; fileRef = 3F38F3732DFA9E0800E0FE41 /* NetworkingCore.generated.swift */; };


Splitting Networking in two requires adding the new NetworkingCore to the Fakes framework.

mokagio · 2025-06-12T07:52:14Z

Fakes/Fakes/Networking.generated.swift

-            gravatarUrl: .fake()
-        )
-    }
-}


All the deletions here went into NetworkingCore.generated.swift

mokagio · 2025-06-12T07:54:42Z

Modules/Package.swift

-        .package(url: "https://github.com/wordpress-mobile/AztecEditor-iOS", from: "1.20.0"),
+        .package(url: "https://github.com/wordpress-mobile/AztecEditor-iOS", revision: "d741e3cfaa74c99ef092e5fddb87d4314b63e3ed"),


This was necessary to compile NetworkingCore without UIKit, see wordpress-mobile/AztecEditor-iOS#1412

Can we do this with an Aztec release? I thought that was part of the development process now, rather than app release...?

Can we do this with an Aztec release?

Yes.

I thought that was part of the development process now, rather than app release...?

Correct. We don't need to be too strict about it, as in "you must ship a new version before using library changes in the app" but it'd be good practice to have something stable and tagged.

The changes in that commit came from wordpress-mobile/AztecEditor-iOS#1412 which was motivated by the modularization work here. At the time, the PR hadn't been merged, but since it has now, I shall follow up with a release.

Thanks!

mokagio · 2025-06-12T08:15:21Z

Modules/Sources/Networking/Tools/Loader.swift

-        return nil
-    }
-
-


This turned out to be unused.

mokagio · 2025-06-12T08:28:30Z

Modules/Sources/Networking/Extensions/URLRequest+Request.swift

+    public func responseDataValidator() -> ResponseDataValidator {
+        PlaceholderDataValidator()
+    }
+}


Not a new file. Just too big of a diff for the engine to follow. Before, it was:

woocommerce-ios/Networking/Sources/Extended/Extensions/URLRequest+Request.swift

Lines 1 to 8 in ae05dab

import Foundation

/// Makes URLRequest conform to Request.

extension URLRequest: Request {

func responseDataValidator() -> ResponseDataValidator {

PlaceholderDataValidator()

}

}

mokagio · 2025-06-12T08:50:12Z

Modules/Sources/Networking/Mapper/MediaListMapper.swift

@@ -1,3 +1,5 @@
+import Foundation
+


As discussed in earlier PRs, when we go from framework to package we lose the Objective-C header file with the customary #import <Foundation/Foundation.h>. This requires files that access foundation types to explicitly import it.

mokagio · 2025-06-18T05:31:45Z

Modules/Sources/NetworkingCore/Extensions/String+HTML.swift

+    public var strippedHTML: String {
+        HTMLParser().parse(self).rawText()
+    }
+}


Looks new, but it's just a big diff for Git to follow:

woocommerce-ios/Networking/Sources/Core/Extensions/String+HTML.swift

Lines 1 to 24 in c8788ef

import Foundation

#if canImport(Aztec)

import class Aztec.HTMLParser

#endif

/// String: HTML Stripping

///

extension String {

/// Returns the HTML Stripped version of the receiver.

///

/// NOTE: I can be very slow ⏳ — using it in a background thread is strongly recommended.

///

public var strippedHTML: String {

#if canImport(Aztec)

HTMLParser().parse(self).rawText()

#else

// This conditional compiling is because Aztec is not available on WatchOS and our watch app does not access this code yet.

// We should consider adding HTML-stripping support when needed.

return self

#endif

}

}

mokagio · 2025-06-18T10:27:23Z

Modules/Sources/Networking/Model/Copiable/Models+Copiable.generated.swift

-        )
-    }
-}
-


All the deleted extensions here went into the new NetworkingCore file.

mokagio · 2025-06-18T10:31:42Z

Modules/Sources/NetworkingCore/Validators/PlaceholderDataValidator.swift

+    public func validate(data: Data) throws {
+        // no-op
+    }
+}


Again, just a diff that Git/GitHub can't resolve as rename + edit. Previous version:

woocommerce-ios/Networking/Sources/Core/Validators/PlaceholderDataValidator.swift

Lines 1 to 9 in 8089b16

import Foundation

/// Placeholder implementation for `ResponseDataValidator`.

///

final class PlaceholderDataValidator: ResponseDataValidator {

func validate(data: Data) throws {

// no-op

}

}

mokagio · 2025-06-18T10:32:37Z

Networking/Networking.xcodeproj/xcshareddata/xcschemes/NetworkingTests.xcscheme

+      buildConfiguration = "Release"
+      revealArchiveInOrganizer = "YES">
+   </ArchiveAction>
+</Scheme>


A temporary crutch. Once the tests will be in the package, too, this will go.

mokagio · 2025-06-18T10:33:40Z

Networking/NetworkingTests/ApplicationPassword/ApplicationPasswordEncoderTests.swift

@@ -1,5 +1,6 @@
 import XCTest
 @testable import Networking
+@testable import NetworkingCore


It was simpler to add @testable import Networking to all the tests than to figure out failed build after failed build which tests needed it and which didn't.

A crude way to avoid access level errors in the tests that use types that are now in NetworkingCore. There are 216 tests in Networking that declare `import Networking`, which from past experience with this kind of changes make me guess that we'd need at least 100 build-fix-repeat iteration to identify all and only those tests that genuinely need the import.

joshheald

💜💜💜💜💜💜💜💜

Thanks for doing this @mokagio – I really appreciate it! Sorry it took me a while to get to it.

I've tested with iOS 17 (iPad), 18 (iPhone), and on the watch (no last-minute crashes from this one!)

I had some intermediate build errors to do with the watch, e.g. this happened when I was building for the iPad after having run the watch app, which was odd:

Could not find module 'Codegen' for target 'arm64-apple-watchos'; found: arm64_32-apple-watchos, at: ~/Library/Developer/Xcode/DerivedData/WooCommerce-aosiyavjlpnpynasfwqzvarsdhnz/Build/Products/Debug-watchos/Codegen.swiftmodule

... but they were sorted with a clean, so I think it's OK.

A few questions inline, but nothing to block merging this.

joshheald · 2025-06-18T13:18:36Z

Fakes/Fakes.xcodeproj/project.pbxproj

 		26CA6D2425F6C87800B01F48 /* Fakes.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; path = Fakes.h; sourceTree = "<group>"; };
 		26CA6D2525F6C87800B01F48 /* Info.plist */ = {isa = PBXFileReference; lastKnownFileType = text.plist.xml; path = Info.plist; sourceTree = "<group>"; };
 		26CA6D2D25F6C8FB00B01F48 /* Fake.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = Fake.swift; sourceTree = "<group>"; };
-		26CA6D3325F6C92100B01F48 /* Networking.framework */ = {isa = PBXFileReference; explicitFileType = wrapper.framework; path = Networking.framework; sourceTree = BUILT_PRODUCTS_DIR; };


Are these just duplicate declarations? I see it's still in the project.

I don't see it on my end:

(In the screenshot, Xcode gives the link to a PR, but you can see my local is on trunk)

These Networking.framework references represented the target dependency. Here's how they looked like:

joshheald · 2025-06-18T13:20:53Z

Modules/Package.swift

-        .package(url: "https://github.com/wordpress-mobile/AztecEditor-iOS", from: "1.20.0"),
+        .package(url: "https://github.com/wordpress-mobile/AztecEditor-iOS", revision: "d741e3cfaa74c99ef092e5fddb87d4314b63e3ed"),


Can we do this with an Aztec release? I thought that was part of the development process now, rather than app release...?

joshheald · 2025-06-18T13:22:02Z

Modules/Package.swift

-                dependencies: ["Codegen"]
+                dependencies: [
+                    "Codegen",
+                    "Networking",


Will we later treat NetworkingCore in the same way, or do we have to treat them differently?

In what sense "treat" @joshheald ?

If you are referring to importing / declaring as a dependency, then Networking makes NetworkingCore available transitively via the @_exported import NetworkingCore directive in Networking.swift.

OK, so we won't need to declare NetworkingCore as a dependency in the package files, but it doesn't have an old-style framework link either? 👍

joshheald · 2025-06-18T13:27:07Z

Modules/Sources/NetworkingCore/Model/MetaContainer.swift

 import Foundation


 // MARK: - MetaContainer: Simple API to query the "Notification Meta" Collection.
 //
-public struct MetaContainer {
+public struct MetaContainer: GeneratedFakeable {


Why did these need to become Fakeable?

🤔 there were a number of things I changed during the transition that then I realized were unnecessary. I thought I had cleaned all of them up, but this could be one that I missed.

Thanks, I'll follow up.

https://linear.app/a8c/issue/AINFRA-806

Not surprised! Thanks for sorting it.

joshheald · 2025-06-18T13:30:10Z

docs/architecture-overview.md

@@ -25,12 +25,12 @@ Throughout the entire architecture design process, we've priorized several key c

 2.  **Separation of concerns**

-        We've emphasized a clean separation of concerns at the top level, by splitting our app into four targets:
+        We've emphasized a clean separation of concerns at the top level, by splitting our app into a number of modules. The main four are:


Good attention to detail!

mokagio force-pushed the ainfra-470-networking-source branch from 3530b6a to aab1f45 Compare June 12, 2025 07:47

mokagio commented Jun 12, 2025

View reviewed changes

mokagio requested a review from a team June 12, 2025 08:50

mokagio self-assigned this Jun 12, 2025

mokagio added this to the 22.7 milestone Jun 12, 2025

mokagio marked this pull request as ready for review June 12, 2025 08:50

mokagio changed the title ~~AINFRA-470 - Make Networking into a Swift package - Sources only~~ Simplify Networking into a Swift package - Sources only Jun 12, 2025

mokagio added the category: tooling Anything that involves building & maintaining the project, including scripts, `Fastfile`, etc. label Jun 12, 2025

mokagio force-pushed the ainfra-470-networking-source branch 3 times, most recently from 253c7db to 64b178a Compare June 16, 2025 09:29

mokagio enabled auto-merge June 16, 2025 09:40

This was referenced Jun 17, 2025

Remove a few dead references and no longer used files #15762

Merged

Simplify Storage into a Swift package - Sources only #15742

Merged

mokagio force-pushed the ainfra-470-networking-source branch 2 times, most recently from 3401deb to af8e65b Compare June 18, 2025 05:23

mokagio commented Jun 18, 2025

View reviewed changes

mokagio added 2 commits June 18, 2025 20:35

AINFRA-470 - Make Networking into a Swift package - Sources only

fba376c

mokagio added 7 commits June 18, 2025 20:35

Add necessary @testable import NetworkingCore to YosemiteTests

d939a7c

Remove unused method from Loader

c6c44bf

Convert Responses in Networking tests to a synced folder

a04ef3a

Remove a commented import

1419b3e

Remove dead references to Networking.framework

99fb51a

Remove Networking scheme from Networking project

ceb66ef

Update architecture docs Networking reference

1c2f4e6

mokagio force-pushed the ainfra-470-networking-source branch from af8e65b to 1c2f4e6 Compare June 18, 2025 10:36

joshheald self-assigned this Jun 18, 2025

joshheald approved these changes Jun 18, 2025

View reviewed changes

mokagio merged commit 96f2279 into trunk Jun 18, 2025
13 checks passed

mokagio deleted the ainfra-470-networking-source branch June 18, 2025 13:38

RafaelKayumov mentioned this pull request Jun 27, 2025

Merge release/22.6 into trunk #15827

Merged

		.package(url: "https://github.com/wordpress-mobile/AztecEditor-iOS", from: "1.20.0"),
		.package(url: "https://github.com/wordpress-mobile/AztecEditor-iOS", revision: "d741e3cfaa74c99ef092e5fddb87d4314b63e3ed"),

	import Foundation

	/// Makes URLRequest conform to Request.
	extension URLRequest: Request {
	func responseDataValidator() -> ResponseDataValidator {
	PlaceholderDataValidator()
	}
	}

	import Foundation

	#if canImport(Aztec)
	import class Aztec.HTMLParser
	#endif

	/// String: HTML Stripping
	///
	extension String {

	/// Returns the HTML Stripped version of the receiver.
	///
	/// NOTE: I can be very slow ⏳ — using it in a background thread is strongly recommended.
	///
	public var strippedHTML: String {
	#if canImport(Aztec)
	HTMLParser().parse(self).rawText()
	#else
	// This conditional compiling is because Aztec is not available on WatchOS and our watch app does not access this code yet.
	// We should consider adding HTML-stripping support when needed.
	return self
	#endif
	}
	}

	import Foundation

	/// Placeholder implementation for `ResponseDataValidator`.
	///
	final class PlaceholderDataValidator: ResponseDataValidator {
	func validate(data: Data) throws {
	// no-op
	}
	}

Simplify Networking into a Swift package - Sources only #15737

Simplify Networking into a Swift package - Sources only #15737

Uh oh!

Conversation

mokagio commented Jun 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Steps to reproduce & Testing information

Screenshots

Uh oh!

dangermattic commented Jun 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

wpmobilebot commented Jun 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mokagio Jun 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

joshheald left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

mokagio commented Jun 12, 2025 •

edited

Loading

dangermattic commented Jun 12, 2025 •

edited

Loading

wpmobilebot commented Jun 12, 2025 •

edited

Loading

mokagio Jun 18, 2025 •

edited

Loading