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

Migrate Turf GeoJSON usage to GeoJSONObject and JSONValue #715

Merged
merged 3 commits into from
Sep 29, 2021

Conversation

1ec5
Copy link
Contributor

@1ec5 1ec5 commented Sep 28, 2021

This PR integrates the GeoJSON API changes in mapbox/turf-swift#154 and Turf v2.0.0-rc.2. Most of the changes are in generated code and in tests, though some examples have also changed in minor ways. There is one breaking change on top of the Turf changes: the generic parameter on the Style.updateGeoJSONSource<T: GeoJSONObject>(withId:geoJSON:) method has been removed. Instead of passing in the expected GeoJSON object type, you perform pattern matching on the return value using case let. (#715)

Depends on mapbox/turf-swift#154.

/cc @mapbox/maps-ios

@1ec5 1ec5 added breaking change ⚠️ If your pull request introduces a breaking change and updates are required when version is published dependencies Pull requests that update a dependency file labels Sep 28, 2021
@1ec5 1ec5 added this to the ga milestone Sep 28, 2021
@1ec5 1ec5 requested a review from macdrevx September 28, 2021 10:28
@1ec5 1ec5 self-assigned this Sep 28, 2021
@1ec5 1ec5 changed the title 1ec5 turf geojson conform 154 Migrate Turf GeoJSON usage to GeoJSONObject and JSONValue Sep 28, 2021
@1ec5 1ec5 force-pushed the 1ec5-turf-geojson-conform-154 branch from 69dcf0e to 02ac3db Compare September 28, 2021 17:21
@1ec5 1ec5 force-pushed the 1ec5-turf-geojson-conform-154 branch from 02ac3db to 11c3965 Compare September 29, 2021 06:58
@1ec5 1ec5 marked this pull request as ready for review September 29, 2021 06:58
@1ec5 1ec5 force-pushed the 1ec5-turf-geojson-conform-154 branch from ed429ce to 2db44f9 Compare September 29, 2021 09:17
@1ec5 1ec5 force-pushed the 1ec5-turf-geojson-conform-154 branch from 70cc847 to 04a8e3a Compare September 29, 2021 11:14
guard case .number(.double(2.0)) = feature.identifier else {
guard feature.identifier == 2.0 else {
Copy link
Contributor

Choose a reason for hiding this comment

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

Love this. Very nice.

Co-authored-by: Andrew Hershberger <andrew.hershberger@mapbox.com>
@1ec5 1ec5 force-pushed the 1ec5-turf-geojson-conform-154 branch from aea129f to 928dc5e Compare September 29, 2021 19:26
@1ec5
Copy link
Contributor Author

1ec5 commented Sep 29, 2021

This build is failing, presumably due to #695 so unrelated to this PR:

CompileSwift normal arm64 (in target 'MobileMetricsTests' from project 'MobileMetrics')
    cd /Users/distiller/project/ios-maps-carbon/benchmarks
/Users/distiller/project/ios-maps-carbon/MobileMetricsTests/gen/ttrcSymbol/ttrcSymbol_navigation_v4.swift:143:40: error: missing arguments for parameters 'stretchX', 'stretchY' in call
                    try! style.addImage(replacementImage, id: imageName)
                                       ^
/Users/distiller/project/ios-maps-carbon/build/SourcePackages/checkouts/mapbox-maps-ios/Sources/MapboxMaps/Style/Style.swift:706:17: note: 'addImage(_:id:sdf:stretchX:stretchY:content:)' declared here
    public func addImage(_ image: UIImage,
                ^
/Users/distiller/project/ios-maps-carbon/MobileMetricsTests/gen/ttrcSymbol/ttrcSymbol_streets_v11.swift:143:40: error: missing arguments for parameters 'stretchX', 'stretchY' in call
                    try! style.addImage(replacementImage, id: imageName)
                                       ^
/Users/distiller/project/ios-maps-carbon/build/SourcePackages/checkouts/mapbox-maps-ios/Sources/MapboxMaps/Style/Style.swift:706:17: note: 'addImage(_:id:sdf:stretchX:stretchY:content:)' declared here
    public func addImage(_ image: UIImage,
                ^
/Users/distiller/project/ios-maps-carbon/benchmarks/MobileMetricsTests/MapsTestCase.swift:138:52: error: type of expression is ambiguous without more context
            if var layer: SymbolLayer = try? style.layer(withId: id) {
                                        ~~~~~~~~~~~^~~~~~~~~~~~~~~~~
/Users/distiller/project/ios-maps-carbon/MobileMetricsTests/gen/ttrcSymbol/ttrcSymbol_satellite_v9.swift:143:40: error: missing arguments for parameters 'stretchX', 'stretchY' in call
                    try! style.addImage(replacementImage, id: imageName)
                                       ^
/Users/distiller/project/ios-maps-carbon/build/SourcePackages/checkouts/mapbox-maps-ios/Sources/MapboxMaps/Style/Style.swift:706:17: note: 'addImage(_:id:sdf:stretchX:stretchY:content:)' declared here
    public func addImage(_ image: UIImage,
                ^
/Users/distiller/project/ios-maps-carbon/MobileMetricsTests/gen/ttrcSymbol/ttrcSymbol_empty_v9.swift:143:40: error: missing arguments for parameters 'stretchX', 'stretchY' in call
                    try! style.addImage(replacementImage, id: imageName)
                                       ^
/Users/distiller/project/ios-maps-carbon/build/SourcePackages/checkouts/mapbox-maps-ios/Sources/MapboxMaps/Style/Style.swift:706:17: note: 'addImage(_:id:sdf:stretchX:stretchY:content:)' declared here
    public func addImage(_ image: UIImage,
                ^
/Users/distiller/project/ios-maps-carbon/MobileMetricsTests/gen/ttrcSymbol/ttrcSymbol_basic_v9.swift:143:40: error: missing arguments for parameters 'stretchX', 'stretchY' in call
                    try! style.addImage(replacementImage, id: imageName)
                                       ^
/Users/distiller/project/ios-maps-carbon/build/SourcePackages/checkouts/mapbox-maps-ios/Sources/MapboxMaps/Style/Style.swift:706:17: note: 'addImage(_:id:sdf:stretchX:stretchY:content:)' declared here
    public func addImage(_ image: UIImage,
                ^

#720 proposes making giving these parameters default arguments so that these calls wouldn’t have to be modified.

@1ec5 1ec5 mentioned this pull request Oct 6, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking change ⚠️ If your pull request introduces a breaking change and updates are required when version is published dependencies Pull requests that update a dependency file
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants