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

Use @_spi(Experimental) for experimental methods #680

Merged
merged 3 commits into from
Sep 15, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ Mapbox welcomes participation and contributions from everyone.
### Breaking changes ⚠️

* `BasicCameraAnimator` now keeps animators alive without the user storing the animator. ([#646](https://github.com/mapbox/mapbox-maps-ios/pull/646/))
* Experimental style APIs are now marked with `@_spi(Experimental)` and the previously used underscore prefixes have been removed. In order to access these methods, use `@_spi(Experimental)` to annotate the import statement for MapboxMaps. ([#680](https://github.com/mapbox/mapbox-maps-ios/pull/680))

### Features ✨ and improvements 🏁

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ public class CircleAnnotationManager: AnnotationManager {
var layer = CircleLayer(id: layerId)
layer.source = sourceId
if shouldPersist {
try style._addPersistentLayer(layer, layerPosition: layerPosition)
try style.addPersistentLayer(layer, layerPosition: layerPosition)
} else {
try style.addLayer(layer, layerPosition: layerPosition)
}
Expand Down Expand Up @@ -145,7 +145,7 @@ public class CircleAnnotationManager: AnnotationManager {
category: "Annotations")
return (key, ["format", ""])
} else {
return (key, Style._layerPropertyDefaultValue(for: .circle, property: key).value)
return (key, Style.layerPropertyDefaultValue(for: .circle, property: key).value)
}
})

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ public class PointAnnotationManager: AnnotationManager {
layer.iconIgnorePlacement = .constant(true)
layer.textIgnorePlacement = .constant(true)
if shouldPersist {
try style._addPersistentLayer(layer, layerPosition: layerPosition)
try style.addPersistentLayer(layer, layerPosition: layerPosition)
} else {
try style.addLayer(layer, layerPosition: layerPosition)
}
Expand Down Expand Up @@ -153,7 +153,7 @@ public class PointAnnotationManager: AnnotationManager {
category: "Annotations")
return (key, ["format", ""])
} else {
return (key, Style._layerPropertyDefaultValue(for: .symbol, property: key).value)
return (key, Style.layerPropertyDefaultValue(for: .symbol, property: key).value)
}
})

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ public class PolygonAnnotationManager: AnnotationManager {
var layer = FillLayer(id: layerId)
layer.source = sourceId
if shouldPersist {
try style._addPersistentLayer(layer, layerPosition: layerPosition)
try style.addPersistentLayer(layer, layerPosition: layerPosition)
} else {
try style.addLayer(layer, layerPosition: layerPosition)
}
Expand Down Expand Up @@ -145,7 +145,7 @@ public class PolygonAnnotationManager: AnnotationManager {
category: "Annotations")
return (key, ["format", ""])
} else {
return (key, Style._layerPropertyDefaultValue(for: .fill, property: key).value)
return (key, Style.layerPropertyDefaultValue(for: .fill, property: key).value)
}
})

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ public class PolylineAnnotationManager: AnnotationManager {
var layer = LineLayer(id: layerId)
layer.source = sourceId
if shouldPersist {
try style._addPersistentLayer(layer, layerPosition: layerPosition)
try style.addPersistentLayer(layer, layerPosition: layerPosition)
} else {
try style.addLayer(layer, layerPosition: layerPosition)
}
Expand Down Expand Up @@ -145,7 +145,7 @@ public class PolylineAnnotationManager: AnnotationManager {
category: "Annotations")
return (key, ["format", ""])
} else {
return (key, Style._layerPropertyDefaultValue(for: .line, property: key).value)
return (key, Style.layerPropertyDefaultValue(for: .line, property: key).value)
}
})

Expand Down
6 changes: 3 additions & 3 deletions Sources/MapboxMaps/Location/LocationStyleProtocol.swift
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
internal protocol LocationStyleProtocol: AnyObject {
func _addPersistentLayer(_ layer: Layer, layerPosition: LayerPosition?) throws
func addPersistentLayer(_ layer: Layer, layerPosition: LayerPosition?) throws
func removeLayer(withId id: String) throws
func layerExists(withId id: String) -> Bool
func setLayerProperties(for layerId: String, properties: [String: Any]) throws
Expand All @@ -12,8 +12,8 @@ internal protocol LocationStyleProtocol: AnyObject {
}

extension LocationStyleProtocol {
internal func _addPersistentLayer(_ layer: Layer, layerPosition: LayerPosition? = nil) throws {
try _addPersistentLayer(layer, layerPosition: layerPosition)
internal func addPersistentLayer(_ layer: Layer, layerPosition: LayerPosition? = nil) throws {
try addPersistentLayer(layer, layerPosition: layerPosition)
}

internal func addImage(_ image: UIImage, id: String, sdf: Bool = false, stretchX: [ImageStretches] = [], stretchY: [ImageStretches] = [], content: ImageContent? = nil) throws {
Expand Down
4 changes: 2 additions & 2 deletions Sources/MapboxMaps/Location/Pucks/Puck2D.swift
Original file line number Diff line number Diff line change
Expand Up @@ -197,7 +197,7 @@ internal extension Puck2D {
}

// Add layer to style
try style._addPersistentLayer(layer)
try style.addPersistentLayer(layer)

locationIndicatorLayer = layer
}
Expand Down Expand Up @@ -237,7 +237,7 @@ internal extension Puck2D {
layer.accuracyRadiusBorderColor = .constant(StyleColor(.lightGray))

// Add layer to style
try style._addPersistentLayer(layer)
try style.addPersistentLayer(layer)

locationIndicatorLayer = layer
}
Expand Down
2 changes: 1 addition & 1 deletion Sources/MapboxMaps/Location/Pucks/Puck3D.swift
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ internal class Puck3D: Puck {

// TODO: On first setup "puck-model does not have a uri"
try? style.addSource(self.modelSource, id: "puck-model-source")
try! style._addPersistentLayer(self.modelLayer)
try! style.addPersistentLayer(self.modelLayer)
}

// Do initial setup
Expand Down
65 changes: 31 additions & 34 deletions Sources/MapboxMaps/Style/Style.swift
Original file line number Diff line number Diff line change
Expand Up @@ -37,26 +37,29 @@ public class Style {
/// - layerPosition: Position at which to add the map.
///
/// - Throws: StyleError or type conversion errors
internal func _addPersistentLayer(_ layer: Layer, layerPosition: LayerPosition? = nil) throws {
internal func addPersistentLayer(_ layer: Layer, layerPosition: LayerPosition? = nil) throws {
// Attempt to encode the provided layer into JSON and apply it to the map
let layerJSON = try layer.jsonObject()
try _addPersistentLayer(with: layerJSON, layerPosition: layerPosition)
try addPersistentLayer(with: layerJSON, layerPosition: layerPosition)
}

/**
:nodoc:
Moves a `layer` to a new layer position in the style.
- Parameter layerId: The layer to move
- Parameter position: The new position to move the layer to

- Throws: `StyleError` on failure, or `NSError` with a _domain of "com.mapbox.bindgen"
- Note: This method is marked as experimental. Annotate the import statement
for `MapboxMaps` with `@_spi(Experimental)` in order to use experimental methods.
*/
public func _moveLayer(withId id: String, to position: LayerPosition) throws {
@_spi(Experimental) public func moveLayer(withId id: String, to position: LayerPosition) throws {
let properties = try layerProperties(for: id)
let isPersistent = try _isPersistentLayer(id: id)
let isPersistent = try isPersistentLayer(id: id)
try removeLayer(withId: id)

if isPersistent {
try _addPersistentLayer(with: properties, layerPosition: position)
try addPersistentLayer(with: properties, layerPosition: position)
} else {
try addLayer(with: properties, layerPosition: position)
}
Expand All @@ -72,7 +75,7 @@ public class Style {
*/
public func layer<T: Layer>(withId id: String) throws -> T {
// swiftlint:disable force_cast
return try _layer(withId: id, type: T.self) as! T
return try layer(withId: id, type: T.self) as! T
// swiftlint:enable force_cast
}

Expand All @@ -87,8 +90,10 @@ public class Style {

- Returns: The fully formed `layer` object of type equal to `type`
- Throws: StyleError or type conversion errors
- Note: This method is marked as experimental. Annotate the import statement
for `MapboxMaps` with `@_spi(Experimental)` in order to use experimental methods.
*/
public func _layer(withId id: String, type: Layer.Type) throws -> Layer {
@_spi(Experimental) public func layer(withId id: String, type: Layer.Type) throws -> Layer {
// Get the layer properties from the map
let properties = try layerProperties(for: id)
return try type.init(jsonObject: properties)
Expand Down Expand Up @@ -125,7 +130,7 @@ public class Style {
/// - Returns:
/// The value of the property in the layer with layerId.
public func layerProperty(for layerId: String, property: String) -> Any {
return _layerProperty(for: layerId, property: property).value
return layerProperty(for: layerId, property: property).value
}

// MARK: - Sources
Expand All @@ -151,7 +156,7 @@ public class Style {
*/
public func source<T: Source>(withId id: String) throws -> T {
// swiftlint:disable force_cast
return try _source(withId: id, type: T.self) as! T
return try source(withId: id, type: T.self) as! T
// swiftlint:enable force_cast
}

Expand All @@ -165,25 +170,16 @@ public class Style {
- Parameter type: The type of the source
- Returns: The fully formed `source` object of type equal to `type`.
- Throws: StyleError or type conversion errors
- Note: This method is marked as experimental. Annotate the import statement
for `MapboxMaps` with `@_spi(Experimental)` in order to use experimental methods.
*/
public func _source(withId id: String, type: Source.Type) throws -> Source {
@_spi(Experimental) public func source(withId id: String, type: Source.Type) throws -> Source {
Comment on lines +173 to +176
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this needs to be experimental anymore. (same for the corresponding layer method above). We could actually knock out another issue related to these APIs at the same time by removing the variants that don't take a type parameter.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That said, let's defer this feedback to our audit of all experimental APIs.

// Get the source properties for a given identifier
let sourceProps = try sourceProperties(for: id)
let source = try type.init(jsonObject: sourceProps)
return source
}

/// Gets the value of style source property.
///
/// - Parameters:
/// - sourceId: Style source identifier.
/// - property: Style source property name.
///
/// - Returns: The value of the property in the source with sourceId.
public func sourceProperty(for sourceId: String, property: String) -> Any {
return _sourceProperty(for: sourceId, property: property).value
}

jmkiley marked this conversation as resolved.
Show resolved Hide resolved
/// Updates the `data` property of a given `GeoJSONSource` with a new value
/// conforming to the `GeoJSONObject` protocol.
///
Expand Down Expand Up @@ -214,7 +210,7 @@ public class Style {
///
/// - Returns: Style light property value.
public func lightProperty(_ property: String) -> Any {
return _lightProperty(property).value
return lightProperty(property).value
}

// MARK: - Terrain
Expand All @@ -240,7 +236,7 @@ public class Style {
///
/// - Returns: Style terrain property value.
public func terrainProperty(_ property: String) -> Any {
return _terrainProperty(property).value
return terrainProperty(property).value
}

// MARK: - Conversion helpers
Expand Down Expand Up @@ -332,19 +328,20 @@ extension Style: StyleManagerProtocol {
}
}

public func _addPersistentLayer(with properties: [String: Any], layerPosition: LayerPosition?) throws {
@_spi(Experimental)
public func addPersistentLayer(with properties: [String: Any], layerPosition: LayerPosition?) throws {
return try handleExpected {
return styleManager.addPersistentStyleLayer(forProperties: properties, layerPosition: layerPosition?.corePosition)
}
}

public func _isPersistentLayer(id: String) throws -> Bool {
@_spi(Experimental) public func isPersistentLayer(id: String) throws -> Bool {
return try handleExpected {
return styleManager.isStyleLayerPersistent(forLayerId: id)
}
}

public func _addPersistentCustomLayer(withId id: String, layerHost: CustomLayerHost, layerPosition: LayerPosition?) throws {
@_spi(Experimental) public func addPersistentCustomLayer(withId id: String, layerHost: CustomLayerHost, layerPosition: LayerPosition?) throws {
return try handleExpected {
return styleManager.addPersistentStyleCustomLayer(forLayerId: id, layerHost: layerHost, layerPosition: layerPosition?.corePosition)
}
Expand Down Expand Up @@ -378,7 +375,7 @@ extension Style: StyleManagerProtocol {

// MARK: - Layer Properties

public func _layerProperty(for layerId: String, property: String) -> StylePropertyValue {
@_spi(Experimental) public func layerProperty(for layerId: String, property: String) -> StylePropertyValue {
return styleManager.getStyleLayerProperty(forLayerId: layerId, property: property)
}

Expand All @@ -388,7 +385,7 @@ extension Style: StyleManagerProtocol {
}
}

public static func _layerPropertyDefaultValue(for layerType: LayerType, property: String) -> StylePropertyValue {
@_spi(Experimental) public static func layerPropertyDefaultValue(for layerType: LayerType, property: String) -> StylePropertyValue {
return StyleManager.getStyleLayerPropertyDefaultValue(forLayerType: layerType.rawValue, property: property)
}

Expand Down Expand Up @@ -434,7 +431,7 @@ extension Style: StyleManagerProtocol {

// MARK: - Source properties

public func _sourceProperty(for sourceId: String, property: String) -> StylePropertyValue {
@_spi(Experimental) public func sourceProperty(for sourceId: String, property: String) -> StylePropertyValue {
return styleManager.getStyleSourceProperty(forSourceId: sourceId, property: property)
}

Expand All @@ -456,7 +453,7 @@ extension Style: StyleManagerProtocol {
}
}

public static func _sourcePropertyDefaultValue(for sourceType: String, property: String) -> StylePropertyValue {
@_spi(Experimental) public static func sourcePropertyDefaultValue(for sourceType: String, property: String) -> StylePropertyValue {
return StyleManager.getStyleSourcePropertyDefaultValue(forSourceType: sourceType, property: property)
}

Expand Down Expand Up @@ -512,7 +509,7 @@ extension Style: StyleManagerProtocol {
}
}

public func _lightProperty(_ property: String) -> StylePropertyValue {
@_spi(Experimental) public func lightProperty(_ property: String) -> StylePropertyValue {
return styleManager.getStyleLightProperty(forProperty: property)
}

Expand All @@ -530,7 +527,7 @@ extension Style: StyleManagerProtocol {
}
}

public func _terrainProperty(_ property: String) -> StylePropertyValue {
@_spi(Experimental) public func terrainProperty(_ property: String) -> StylePropertyValue {
return styleManager.getStyleTerrainProperty(forProperty: property)
}

Expand All @@ -548,7 +545,7 @@ extension Style: StyleManagerProtocol {
}
}

public func _setCustomGeometrySourceTileData(forSourceId sourceId: String, tileId: CanonicalTileID, features: [Turf.Feature]) throws {
@_spi(Experimental) public func setCustomGeometrySourceTileData(forSourceId sourceId: String, tileId: CanonicalTileID, features: [Turf.Feature]) throws {
let mbxFeatures = features.compactMap { MapboxCommon.Feature($0) }
return try handleExpected {
return styleManager.setStyleCustomGeometrySourceTileDataForSourceId(sourceId, tileId: tileId, featureCollection: mbxFeatures)
Expand All @@ -573,7 +570,7 @@ extension Style: StyleManagerProtocol {
extension Style {
internal func sourceAttributions() -> [String] {
return allSourceIdentifiers.compactMap {
sourceProperty(for: $0.id, property: "attribution") as? String
sourceProperty(for: $0.id, property: "attribution").value as? String
}
}
}
Expand Down
Loading