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

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

Closed
1ec5 opened this issue Apr 21, 2022 · 6 comments
Labels
backwards incompatible changes that break backwards compatibility of public API bug build op-ex Refactoring, Tech Debt or any other operational excellence work. release blocker
Milestone

Comments

@1ec5
Copy link
Contributor

1ec5 commented Apr 21, 2022

#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 #655 (comment)? Either way, we need to issue the fix in v2.4.1 per semver rules.

/cc @mapbox/navigation-ios

@1ec5 1ec5 added bug build op-ex Refactoring, Tech Debt or any other operational excellence work. release blocker labels Apr 21, 2022
@1ec5 1ec5 added this to the v2.4 milestone Apr 21, 2022
@1ec5
Copy link
Contributor Author

1ec5 commented Apr 21, 2022

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

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

@Subhashini2610
Copy link

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

@Andreas4242
Copy link

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

@bamx23
Copy link
Contributor

bamx23 commented Apr 23, 2022

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
Copy link
Contributor Author

1ec5 commented Apr 26, 2022

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.

@1ec5 1ec5 pinned this issue Apr 26, 2022
@1ec5 1ec5 added the backwards incompatible changes that break backwards compatibility of public API label Apr 26, 2022
@jill-cardamon
Copy link
Contributor

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backwards incompatible changes that break backwards compatibility of public API bug build op-ex Refactoring, Tech Debt or any other operational excellence work. release blocker
Projects
None yet
Development

No branches or pull requests

5 participants