Extract `HTMLParser` in dedicated library #1412

mokagio · 2025-05-28T20:11:46Z

Fixes https://linear.app/a8c/issue/AINFRA-639

The code is already in use in the WooCommerce iOS watchOS app, see woocommerce/woocommerce-ios@ec7a641

I decide to extract the files under the standard Sources/ and Tests/ root folders that SwiftPM expects. To keep the Package.swift and podspecs consistent, I moved all the other sources and tests there, too.

I updated the plumbing in the example app and ran it locally to ensure it still works, for what is worth.

I have considered if this change warrants release notes and have added them to the appropriate section in the CHANGELOG.md if necessary. – N.A. many files were moved, but the source change was minimal.

There are likely more files that could go into this target, but in the context of getting `HTMLParser` accessing by WooCommerce on watchOS (without UIKit) it's best to move forward without clearly separating the tests. The tests in Aztec already exercise the logic in the package anyway, so we remain covered.

No idea how the resizing might be failing...

mokagio · 2025-06-04T02:16:55Z

CI is failing with

Compiling for iOS 12.0, but module 'Aztec' has a minimum deployment target of iOS 13.0: /Users/builder/Library/Developer/Xcode/DerivedData/App-etmfcijrdcehzwescvrvmhwzlcrt/Build/Products/Debug-iphonesimulator/WordPress-Aztec-iOS/Aztec.framework/Modules/Aztec.swiftmodule/arm64-apple-ios-simulator.swiftmodule

but I don't get this locally 🤔

Update: Gave up on trying to fix this. Simply bumped everything to iOS 13 to satisfy it. bd3eacc

This aligns it with the pod and package configuration. This changed is done in the context of CI failing with - ERROR | xcodebuild: /opt/ci/builds/builder/automattic/aztec-editor-ios/Tests/AztecTests/Extensions/ArrayHelperTests.swift:2:18: error: compiling for iOS 12.0, but module 'Aztec' has a minimum deployment target of iOS 13.0: /Users/builder/Library/Developer/Xcode/DerivedData/App-etmfcijrdcehzwescvrvmhwzlcrt/Build/Products/Debug-iphonesimulator/WordPress-Aztec-iOS/Aztec.framework/Modules/Aztec.swiftmodule/arm64-apple-ios-simulator.swiftmodule However, it's a wild guess, as I don't see how CocoaPods would pick iOS 13 as the deployment target from the Example app, when none of its paths are given to CocoaPods. https://buildkite.com/automattic/aztec-editor-ios/builds/211/steps/canvas?jid=019738a1-cb58-4c2d-84de-d8aa8f76a57b

After all, all clients of this app already support above iOS 13. Hopefully this will address the 12 vs 13 issue we're seeing in CI. https://buildkite.com/automattic/aztec-editor-ios/builds/211/steps/canvas?jid=019738a1-cb58-4c2d-84de-d8aa8f76a57b

mokagio · 2025-06-05T04:51:24Z

.buildkite/shared-pipeline-vars

 # The ~> modifier is not currently used, but we check for it just in case
 XCODE_VERSION=$(sed -E 's/^~> ?//' .xcode-version)
-CI_TOOLKIT_PLUGIN_VERSION="3.8.0"
+CI_TOOLKIT_PLUGIN_VERSION="5.3.1"


Routine update.

mokagio · 2025-06-05T04:52:58Z

.buildkite/shared-pipeline-vars

+CI_TOOLKIT_PLUGIN_VERSION="5.3.1"

-export IMAGE_ID="xcode-$XCODE_VERSION-macos-14.7.1-v1"
+export IMAGE_ID="xcode-$XCODE_VERSION"


Suffix removed because I also updated the tooling to 16.4 which does not need it.

mokagio · 2025-06-05T04:59:55Z

Package.swift

 let package = Package(
    name: "WordPress-AztecEditor-iOS",
-    platforms: [.iOS(.v11)],
+    platforms: [.iOS(.v13)],


The reason for the version bump is layered.

This PR extracts HTMLParser as a standalone library so that the WooCommerce watchOS app can use it via SwiftPM, since watchOS does not support UIKit.

I knew the restructuring works when the library is fetched via SwiftPM because both the example app and Woo build and run.

But the only way I had to verify the CocoaPods setup, which required a few conditional imports (see the rest of the diff) was to run the unit tests as part of the spec verification.

But in CI, the tests failed with:

- ERROR | xcodebuild: /opt/ci/builds/builder/automattic/aztec-editor-ios/Tests/AztecTests/Extensions/ArrayHelperTests.swift:2:18: error: compiling for iOS 12.0, but module 'Aztec' has a minimum deployment target of iOS 13.0: /Users/builder/Library/Developer/Xcode/DerivedData/App-fnvsuwzsjydtrdcwmvzmpjrcydqz/Build/Products/Debug-iphonesimulator/WordPress-Aztec-iOS/Aztec.framework/Modules/Aztec.swiftmodule/arm64-apple-ios-simulator.swiftmodule

However on my local machine the same bundle exec pod lib lint command passes.

After a number of unsuccessful attempts to address the CI issue I simply decided to bump the minimum deployment target and move on. After all, all clients of this app already expect version way above iOS 13.

mokagio · 2025-06-11T01:26:41Z

@AliSoftware @iangmaia @twstokes could I ask you for a review?

The code is already in use in WooCommerce, referenced by a commit, but it would be good to merge this. Publishing a new tag would also be neat.

AliSoftware

Ran bundle exec fastlane test and passed as expected (well after I updated TEST_RUNTIME = 'iOS 18.3' because I didn't have the 18.2 runtime installed on my Mac with the Xcode versions I have and didn't want to lose time downloading the whole runtime just for that while I won't use it afterwards and it'd take lots of disk space)

Left a couple of questions regarding the #if SWIFT_PACKAGE imports though, so I disabled auto-merge so you can review them first if needed, but approving to unblock.

AliSoftware · 2025-06-11T09:14:31Z

.../AztecTests/Converters/AttributesToStringAttributes/BoldElementAttributeConverterTests.swift

+#if SWIFT_PACKAGE
+@testable import HTMLParser
+#endif


I'm unsure why this @testable import has to be gated by #if SWIFT_PACKAGE (while other imports don't)? 🤔

It has to do with the difference between how CocoaPods and SwiftPM build the library, or rather libraries.

In SwiftPM, we now have the standalone HTMLParser library and Aztec depends on it.

But in CocoaPods, I didn't expose HTMLParser. That's a bit of an inconsistency, I'll admit. But it seemed fine to only work in the context of SwiftPM, between the state of CocoaPods and the fact that this change was just to unblock SwiftPM in Woo.

But this situation resulted in a few build differences. The HTMLParser sources are part of Aztec when building with CocoaPods, but when building for SwiftPM (the default in Xcode and xcodebuild/CLI) they aren't.

Ok I think I see (even though this is a bit much for my mushy Friday evening brain to take on fully 😅)

This seems like an important explanation, and something someone other that you (who worked on this migration directly and are thus familiar with the kind of hybrid/unbalanced setup you had to end up with) would require some digging to understand this subtlety and the reason behind it.

As such, I'll suggest to document it somewhere in the code. Like maybe a code comment explaining all that in the key/main/root place in the codebase where this #if case happens for the first time, then code comments next to all other #if you had to similarly add just pointing to that longer comment/explanation (to avoid copy/pasting same long explanatory comment at every single place)? wdyt?

AliSoftware · 2025-06-11T09:16:09Z

Tests/AztecTests/Importer/InAttributeConverterTests.swift

+#if SWIFT_PACKAGE
+@testable import HTMLParser
+#else
+@testable import Aztec
+#endif


This one is even more confusing to me… why do we import one package in one context (#if SWIFT_PACKAGE) and a different one in another? 🤔

After the explanation above, this should be clearer.

This test only exercises code that is part of the HTMLParser library. In SwiftPM that is a standalone module. But when building for CocoaPods, that's part of Aztec.

Follow up to the conversation on #1412 (comment)

mokagio added 3 commits May 29, 2025 06:08

Add .editorconfig to maintain formatting across editors

43ad96b

Isolate HTMLParser in dedicated target

ba1cea5

mokagio force-pushed the mokagio/extract-html-parser branch 3 times, most recently from 4465a5c to ade72c7 Compare June 3, 2025 08:29

Restructure entire project to align with Swift package manager

1b98c11

mokagio force-pushed the mokagio/extract-html-parser branch from ade72c7 to 1b98c11 Compare June 3, 2025 08:37

mokagio added 5 commits June 3, 2025 18:41

Fix an incorrect SwiftPM macro

3f4ffb0

Could it be that the UIImageResizeTests failure was due to iOS 18.2?

13ed11b

Add tests to CocoaPods Aztec spec, but they don't pass either

f20d5a0

No idea how the resizing might be failing...

Restore skipping tests that got re-enabled in moving files

fa3ea7a

Use an Xcode workspace to link the packages with the example app

1854660

mokagio force-pushed the mokagio/extract-html-parser branch from ab93da2 to 1854660 Compare June 4, 2025 01:50

mokagio added 5 commits June 5, 2025 11:53

Set iOS 12 as the platform in the Package

262ccdf

Use latest CocoaPods

88fbb9a

Use Xcode 16.4 in CI

98a335e

Use latest CI toolkit, from 3.8.0 to 5.3.1

38c3770

mokagio force-pushed the mokagio/extract-html-parser branch from bfe41ed to 38c3770 Compare June 5, 2025 02:25

mokagio commented Jun 5, 2025

View reviewed changes

mokagio marked this pull request as ready for review June 5, 2025 05:10

mokagio requested a review from a team June 5, 2025 05:15

mokagio enabled auto-merge June 5, 2025 22:51

AliSoftware disabled auto-merge June 11, 2025 09:24

AliSoftware approved these changes Jun 11, 2025

View reviewed changes

mokagio mentioned this pull request Jun 12, 2025

Simplify Networking into a Swift package - Sources only woocommerce/woocommerce-ios#15737

Merged

1 task

mokagio merged commit 41327b9 into develop Jun 16, 2025
4 checks passed

mokagio deleted the mokagio/extract-html-parser branch June 16, 2025 04:17

mokagio mentioned this pull request Jun 16, 2025

Add explanatory comments to the conditional HTMLParser imports #1413

Merged

mokagio added a commit that referenced this pull request Jun 16, 2025

Add explanatory comments to the conditional HTMLParser imports (#1413)

2aade45

Follow up to the conversation on #1412 (comment)

Extract HTMLParser in dedicated library #1412

Extract HTMLParser in dedicated library #1412

Uh oh!

Conversation

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

Uh oh!

Uh oh!

mokagio commented Jun 4, 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!

mokagio commented Jun 11, 2025

Uh oh!

AliSoftware 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!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Extract `HTMLParser` in dedicated library #1412

Extract `HTMLParser` in dedicated library #1412

mokagio commented May 28, 2025 •

edited

Loading

mokagio commented Jun 4, 2025 •

edited

Loading