mapbox / mapbox-directions-swift

Traffic-aware directions and map matching in Swift on iOS, macOS, tvOS, watchOS, and Linux
https://www.mapbox.com/navigation/
ISC License
183 stars 88 forks source link

Compiler error subclassing RouteOptions — required initializer must be provided by subclass #684

Closed 1ec5 closed 2 years ago

1ec5 commented 2 years ago

655 changed the RouteOptions(waypoints:profileIdentifier:) initializer to take an additional queryItems parameter. Even though a default argument was provided, a subclass such as the navigation SDK’s NavigationRouteOptions would become incompatible and fail to build (albeit with a fix-it suggestion) because the signature differs:

/path/to/mapbox-navigation-ios/Sources/MapboxCoreNavigation/NavigationRouteOptions.swift:70:1: 'required' initializer 'init(waypoints:profileIdentifier:queryItems:)' must be provided by subclass of 'RouteOptions'

This change probably got missed in code review because it was marked public rather than open, but it is required nonetheless. It would’ve been caught by an API diffing tool: #683.

Can we get around this issue by adding a convenience initializer with the old set of arguments, or do we need to revisit https://github.com/mapbox/mapbox-directions-swift/pull/655#discussion_r819995251? Either way, we need to issue the fix in v2.4.1 per semver rules.

/cc @mapbox/navigation-ios

1ec5 commented 2 years ago

It would’ve been caught by an API diffing tool: #683.

677 implemented this tool, but it was too late for #655.

Subhashini2610 commented 2 years ago

Facing the same issue here. When can we get an update for this?

Andreas4242 commented 2 years ago

I have the same issue after updating the POD from MapboxDirections (2.3.0) to 2.4.0.

bamx23 commented 2 years ago

We are going to make patch releases for Navigation SDKs early next week. As a workaround, you can explicitly specify MapboxDirections 2.3.0 in your dependency managers or update Navigation SDK to 2.4.0.

1ec5 commented 2 years ago

Either way, we need to issue the fix in v2.4.1 per semver rules.

We’re in a catch-22 because we caught this issue late enough that some downstream packages, including MapboxCoreNavigation v2.4.0, already depend on the initializer signature in MapboxDirections v2.4.0. There’s no way to make the class simultaneously compatible with subclasses expecting either signature. Whether we maintain the current behavior or revert to the previous behavior, we’ll break compatibility with the NavigationRouteOptions and NavigationMatchOptions implementations in one set of versions or another.

To try to make the situation as painless as possible, all things considered, we’ll issue patch releases of MapboxCoreNavigation v2.1.x, v2.2.x, and v2.3.x that require MapboxDirections v2.4.0 instead of older versions. These MapboxCoreNavigation releases are being prepared in mapbox/mapbox-navigation-ios#3849, mapbox/mapbox-navigation-ios#3848, and mapbox/mapbox-navigation-ios#3846.

Unfortunately, if you’ve defined your own direct subclass of RouteOptions or MatchOptions, we’re unable to ensure backwards compatibility with that subclass once you upgrade to MapboxDirections v2.4.0. You’ll need to update your override of RouteOptions(waypoints:profileIdentifier:) to RouteOptions(waypoints:profileIdentifier:queryItems:), taking an extra third parameter. The method signature should look like this:

public required init(waypoints: [Waypoint], profileIdentifier: ProfileIdentifier? = .automobileAvoidingTraffic, queryItems: [URLQueryItem]? = nil)

Xcode will offer a fix-it suggestion for this new signature.

Going forward, we’ve taken steps to make sure we catch similar backwards compatibility issues before they make it into a final release of MapboxDirections.

jill-cardamon commented 2 years ago

Patch releases of MapboxCoreNavigation can be found here: v2.1.2, v2.2.1, and v2.3.2.