Make MGLMapView.style property nullable

[1ec5]

MGLMapView’s style property is now nullable (optional in Swift) and is set to nil while the style loads and in the event that the style has failed to load. The styleURL property remains null-resettable (implicitly-unwrapped-optional in Swift) as before.

Essentially, the API as it exists has a trap: the developer is tempted to have runtime styling code run synchronously after initializing the map view, for example in -viewDidLoad. Unfortunately, any runtime styling API operations they perform while the style loads gets wiped out. Instead, they must wait until the style has finished loading, in -[MGLMapViewDelegate mapView:didFinishLoadingStyle:].

We want developers to have a way of knowing whether the style has finished loading so they can easily find out why their runtime styling code isn’t working as expected. The problem with adding a warning in -style, as #7639 does, is that we’d assume the developer intends to use the MGLStyle object they obtain from MGLMapView.style. So if the developer calls the style getter to check whether the style has loaded, they get console spew. It turns out that, if the developer adds a key-value observer (or binding) to the style key, the KVO system calls the style getter even just before the style is set (as a side effect of -willChangeValueForKey:).

This PR avoids that whole mess by preventing the developer from accessing MGLStyle until MGLStyle can be used fruitfully. Swift developers must acknowledge the possibility that style isn’t set yet. Unfortunately, Objective-C developers will have little indication as to the problem without stepping in the debugger.

Fixes #7512.

/ref #7639 (comment)
/cc @boundsj @ericrwolfe @incanus @frederoni @friedbunny @jfirebaugh

[mention-bot]

@1ec5, thanks for your PR! By analyzing this pull request, we identified @boundsj, @incanus and @friedbunny to be potential reviewers.

[1ec5]

Missed a couple files due to a merge conflict. Rebased and force-pushed the proper fix.

[1ec5]

@boundsj pointed out in chat that #6386 will provide a convenient alternative to implementing -mapView:didFinishLoadingStyle:, in the event that the developer would rather perform runtime styling operations inside -viewDidLoad.

[boundsj]

I think this looks good and makes a lot of sense 👍

I'm not sure about the macOS test failure (especially after efcf574)

[boundsj]

👍

[1ec5]

2b743bc fixes the macOS test failure by creating MGLMapView programmatically for both platforms so that the initializer sets the intended style URL instead of the default Streets URL.

[1ec5]

One downside is that this change, in combination with #6386, would lead to a weird scenario where you could set the style property but getting it right after would return a different value (nil). Cocoa APIs tend to address this problem using a loading or loaded property (which we’d put on MGLStyle itself) but making the property itself non-nullable.

[incanus]

I have mixed feelings about making style optional due to the verbosity of ? and ! that would be introduced in runtime styling code.

So if the developer calls the style getter to check whether the style has loaded, they get console spew.

Is there a way that we can check whether style is being messaged or not to avoid this? For example, a gate on its methods?

That way, if it's just the RHS of an assignment or a conditional check, it wouldn't trigger the console warning.

[1ec5]

I have mixed feelings about making style optional due to the verbosity of ? and ! that would be introduced in runtime styling code.

Agreed, it can get repetitive. Fortunately, any code inside MGLMapViewDelegate.mapView(_:didFinishLoading:) can take advantage of the style argument, which isn’t optional. Code located elsewhere, such as in an action method, can mitigate the repetitiveness by unwrapping the optional upfront:

guard let style = mapView.style else { return }

or:

let style = mapView.style!

Is there a way that we can check whether style is being messaged or not to avoid this? For example, a gate on its methods?

Yes, it would be possible to warn or assert if any method of MGLStyle is called before it loads. That would be closer to the loading pattern described in #7664 (comment). However, in that case, aren’t we really using MGLStyle as a placeholder for nil?

[1ec5]

Some integration tests started failing as soon as this PR landed, even though it’s fine on my machine and was fine on the branch. A fix is in #7684.