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

Demystifying the black magic of addLayer #5728

Closed
jingsam opened this issue Nov 21, 2017 · 9 comments
Closed

Demystifying the black magic of addLayer #5728

jingsam opened this issue Nov 21, 2017 · 9 comments

Comments

@jingsam
Copy link
Contributor

jingsam commented Nov 21, 2017

I know addLayer have a black magic dealing with source.

When you want to add a layer and related source to the map, you are supposed to do like this:

// 1. add a source
map.addSource('aaa', source)

// 2. add a layer ref to the source
map.addLayer({
  id: 'bbb',
  source: 'aaa'
})

In fact, you can do like this:

map.addLayer({
  id: 'bbb',
  source: {
     type: 'geojson',
     data: './a.geojson'
  }
})

Why you can do that? When you inline a source definition in layer, the addLayer will create a new source with same id of layer.

The docs says:

Adds a [Mapbox style layer] to the map's style.

The proble is:

  1. inline source in layer style is not confront to mapbox-style-spec
  2. if we indeed allow inline source in layer, we should make it explict in the docs, as to avoid some confusion in edge case such as conflict with the existing sources.
@mollymerp
Copy link
Contributor

For context, support for inline sources was added in #3861

@jfirebaugh
Copy link
Contributor

I think inline sources might have been an API design mistake. It was probably responsible for the confusion in #5440.

@andrewharvey
Copy link
Collaborator

I think inline sources might have been an API design mistake. It was probably responsible for the confusion in #5440.

Personally I avoid using inline sources as I feel it makes the code less readable, more confusing and less refactorable, especially when you start having multiple layers built from the 1 source.

I go so far as un-inlining sources when copying the examples to build upon. I'm not advocating removing inline source support, but I'd be 👍 for changing the examples back to un-inlined sources. I feel this is especially important for the examples which need to be immediately and easily understood and be a clear extension of the GL JS architecture where layers and sources are probably the most important concepts.

@stevage
Copy link
Contributor

stevage commented Nov 29, 2017

Huh. I wasn't really aware you could inline sources like that. Although it looks convenient, I think I'd agree with Andrew that it could get confusing. One thing I like about the Mapbox-GL-JS API is that there is generally only one way to do things, and it has resisted many temptations to add too many conveniences and syntactic sugars etc. (For contrast, try the Tangram API - eep!)

@jfirebaugh jfirebaugh mentioned this issue Feb 14, 2018
1 task
@pathmapper
Copy link
Contributor

@andrewharvey
Copy link
Collaborator

Thanks @pathmapper. Given all this and given your comment @jfirebaugh "I think inline sources might have been an API design mistake". Should we change the examples back to the original style?

@jimutt
Copy link

jimutt commented Feb 12, 2019

Sorry for necroing this old issue, but I'd like to bring some life into this discussion again. As a new user coming to Mapbox GL I've just spent some time reading various issues, SO posts and searching the documentation in order to understand my problems with handling layers removal (and some other related stuff).

My issues all seems to originate from my lack of understanding of the layer/source relation and the different way of declaring sources. I've started using Mapbox GL through the examples section of the documentation and they are pretty clear about how to achieve the specific functionality that's in the scope of a specific example. Though I believe they are lacking when it comes to best practices recommendations / more general usage guidelines.

After reaching some understanding about the topic I think I agree with @jfirebaugh that the current API design could be considered an unfortunate mistake and that it should be reconsidered. Though it could help a lot for new users like me if the current documentation examples are improved in order to avoid this confusion.

@stevage
Copy link
Contributor

stevage commented Feb 12, 2019

Having done my own further thinking around this (especially with respect to my library, mapbox-gl-utils), I think if anything the inlining is inside-out. A layer definition should not contain a source definition. But a source definition could contain one or more layer definitions.

Anyway, my vote would be for deprecating this feature, and removing all reference to it from docs and examples.

@andrewharvey
Copy link
Collaborator

It seems the consensus here is at a minimum do away with inline sources in the examples. @mourner What do you think about changing the examples back to use distinct addSource and addLayer?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

8 participants