Skip to content
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

Make OpenAPIRuntime depend on FoundationEssentials only #732

Open
weissi opened this issue Feb 8, 2025 · 7 comments
Open

Make OpenAPIRuntime depend on FoundationEssentials only #732

weissi opened this issue Feb 8, 2025 · 7 comments
Labels
area/runtime Affects: the runtime library. kind/enhancement Improvements to existing feature. status/blocked Waiting for another issue.
Milestone

Comments

@weissi
Copy link
Member

weissi commented Feb 8, 2025

Motivation

FoundationEssentials contains only the more modern types and comes in (on non-Darwin) much more lightweight than Foundation which also includes a lot of legacy stuff from swift-corelibs-foundation.

Ideally, the OpenAPIRuntime should be happy with just FoundationEssentials which contains everything that I think it needs (JSON, Date, URL, Data, ...).

Proposed solution

Replace all import Foundations with import FoundationEssentials.

Alternatives considered

No response

Additional information

No response

@weissi weissi added kind/feature New feature. status/triage Collecting information required to triage the issue. labels Feb 8, 2025
@czechboy0
Copy link
Contributor

We'll need to import conditionally, as FoundationEssentials isn't available on all platforms.

The mechanism for doing this without breaking API will likely be to add a default trait with the "full Foundation" imports, allowing adopters to turn off default traits and only get the code that requires FoundationEssentials. That'll require Swift 6.1.

@czechboy0 czechboy0 added area/runtime Affects: the runtime library. kind/enhancement Improvements to existing feature. status/blocked Waiting for another issue. and removed kind/feature New feature. status/triage Collecting information required to triage the issue. labels Feb 8, 2025
@czechboy0 czechboy0 added this to the Post-1.0 milestone Feb 8, 2025
@weissi
Copy link
Member Author

weissi commented Feb 10, 2025

We'll need to import conditionally, as FoundationEssentials isn't available on all platforms.

The mechanism for doing this without breaking API will likely be to add a default trait with the "full Foundation" imports, allowing adopters to turn off default traits and only get the code that requires FoundationEssentials. That'll require Swift 6.1.

You should just write

#if canImport(FoundationEssentials)
import FoundationEssentials
#else
import Foundation
#endif

Of course this will only work iff you haven't actually used any legacy Foundation types in your API.

@czechboy0
Copy link
Contributor

Yeah that's the idea, but we can't do it without traits, otherwise it's a breaking change: apple/swift-metrics#158 (comment)

@weissi
Copy link
Member Author

weissi commented Feb 10, 2025

Yeah that's the idea, but we can't do it without traits, otherwise it's a breaking change: apple/swift-metrics#158 (comment)

Whilst it's true that this is observable by others in theory, I don't think it applies to OpenAPIRuntime. It's just not as core of a package as swift-metrics & swift-crypto. Much much fewer people will depend on it and we can audit it by searching github.

Furthermore, OpenAPI also has the transports, both of AHC / URLSession also import Foundation which means the potential for breakage should be very very small.

I really don't think it's worth using traits for this theoretical breakage. I doubt there is any real world code that relies on import Foundation coming from a place that has import OpenAPIRuntime.

@czechboy0
Copy link
Contributor

@weissi
Copy link
Member Author

weissi commented Feb 10, 2025

apple/swift-metrics#158 (comment)

OpenAPIRuntime is not SwiftMetrics. Almost everywhere also import OpenAPI{URLSession,AsyncHTTPClient. Tbh in my github search I couldn't find any mention that doesn't either also import a transport or Foundation in the lines around it...

If you really feel you can't do that (which is think is just making your own life hard), the way to solve it is

  1. Make a new module OpenAPIRuntimeEssentials and move everything in there
  2. Have a single file in OpenAPIRuntime which import Foundation; @_exported import OpenAPIRuntimeEssentials.

That prevents bugs and accidental legacy usage whilst keeping existing behaviours.

@Lukasa
Copy link

Lukasa commented Feb 10, 2025

The above is my proposed plan for swift-asn1 and swift-certificates, incidentally, which I expect to do in the next couple of weeks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/runtime Affects: the runtime library. kind/enhancement Improvements to existing feature. status/blocked Waiting for another issue.
Projects
None yet
Development

No branches or pull requests

3 participants