{"action":"created","issue":{"url":"https://api.github.com/repos/mapbox/mapbox-gl-js/issues/2667","repository_url":"https://api.github.com/repos/mapbox/mapbox-gl-js","labels_url":"https://api.github.com/repos/mapbox/mapbox-gl-js/issues/2667/labels{/name}","comments_url":"https://api.github.com/repos/mapbox/mapbox-gl-js/issues/2667/comments","events_url":"https://api.github.com/repos/mapbox/mapbox-gl-js/issues/2667/events","html_url":"https://github.com/mapbox/mapbox-gl-js/pull/2667","id":158458934,"number":2667,"title":"Refactor Source interface to allow/prepare for extensibility","user":{"login":"anandthakker","id":4974151,"avatar_url":"https://avatars.githubusercontent.com/u/4974151?v=3","gravatar_id":"","url":"https://api.github.com/users/anandthakker","html_url":"https://github.com/anandthakker","followers_url":"https://api.github.com/users/anandthakker/followers","following_url":"https://api.github.com/users/anandthakker/following{/other_user}","gists_url":"https://api.github.com/users/anandthakker/gists{/gist_id}","starred_url":"https://api.github.com/users/anandthakker/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/anandthakker/subscriptions","organizations_url":"https://api.github.com/users/anandthakker/orgs","repos_url":"https://api.github.com/users/anandthakker/repos","events_url":"https://api.github.com/users/anandthakker/events{/privacy}","received_events_url":"https://api.github.com/users/anandthakker/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":34,"created_at":"2016-06-03T21:32:57Z","updated_at":"2016-07-01T19:04:32Z","closed_at":null,"pull_request":{"url":"https://api.github.com/repos/mapbox/mapbox-gl-js/pulls/2667","html_url":"https://github.com/mapbox/mapbox-gl-js/pull/2667","diff_url":"https://github.com/mapbox/mapbox-gl-js/pull/2667.diff","patch_url":"https://github.com/mapbox/mapbox-gl-js/pull/2667.patch"},"body":"Replaces #2648 \r\n\r\n - Inverts `TilePyramid` and `Source` dependencies.  `TilePyramid` now essentially serves as a common wrapper class for dealing with sources.\r\n\r\n - Renames `TilePyramid` to `SourceCache`.  This is partly aspirational, and partly to make it more reasonable to have things called `source` that are actually TilePyramids 😁 \r\n\r\n - Eliminates the ability to create a `Source` independently of a `Map` instance - i.e., no more `new mapboxgl.GeoJSONSource()`; sources must be created with `map.addSource(id, json)`. \r\n\r\n - Eliminates bucketStats, as it is no longer needed (so @lucaswoj assures me)\r\n\r\n- Source _types_ are now essentially:\r\n```js\r\n{\r\n  create: (id, options, dispatcher) => Source\r\n  workerSourceURL: string | undefined\r\n}\r\n```\r\n\r\n- The `Source` provided by the factory function is an object implementing:\r\n\r\n```js\r\nid: string\r\ntileSize: number\r\nminzoom: number\r\nmaxzoom: number\r\nroundZoom: boolean\r\nreparseOverscaled: boolean\r\nisTileClipped: boolean\r\n\r\nloadTile: (tile: Tile, cb: (err, ?data) => void) => void\r\nabortTile: (tile: Tile) => void\r\nunloadTile: (tile: Tile) => void\r\nserialize: () => Object // plain JS\r\n?prepare: () => void\r\n\r\n// @fires `load` to indicate source data has been loaded, so that it's okay to call `loadTile`\r\n// @fires `change` to indicate source data has changed, so that any current caches should be flushed \r\n```\r\n\r\n - `WorkerSourceURL` points to a script, which, when executed in the worker thread, must call `self.registerWorkerSource(workerSource: WorkerSource)`, where\r\n\r\n```js\r\ntype WorkerSource = {\r\n   loadTile: (params, callback: (error, { tile: VectorTile, rawTileData: Buffer}) => void) => void\r\n}\r\n```\r\n\r\nThe `loadTile` method is used by the worker when its `'load tile'` target is invoked with a `type: 'source-type'` parameter (e.g. `type: 'geojson'`).  Any other methods on `workerSource` can be targeted by the main-thread `Source` via `dispatcher.send('source-type.methodname', params, callback)`.\r\n\r\n\r\nQuestions:\r\n - [ ] Does the above design have broad (enough) approval?\r\n - [x] The `Tile` objects are used by various source types to maintain tile-specific state; ideally, this implicit contract between `TilePyramid` and `Source` should be made explicit in the Source interface... but I think this is perhaps out of scope for this particular PR - perhaps this would be naturally addressed in #2703, or else could be ticketed separately?\r\n - [x] I'm not sure what the `data` yielded by `loadTile` should look like.  Currently, the only thing I can see being needed by code outside of the particular source is `bucketStats`, which the pyramid uses to emit the `tile.stats` event.  @lucaswoj can you weigh on in this?\r\n\r\nRemaining Tasks:\r\n - [x] Video source - @lucaswoj if you can help me get this working, I'm assuming I can probably then manage Image source.\r\n - [x] Image source\r\n - [x] Settle Geojson Source design\r\n   - [supercluster: separate source type, or option for geojson source?](https://github.com/mapbox/mapbox-gl-js/pull/2667#issuecomment-224285621)\r\n   - Simplify the geojson source's WorkerSource so that it is (a) as clean/simple as possible, but (b) nicely supports 3rd party \"geojson-like\" source types\r\n - [x] Document the `Source` interface\r\n - [x] Document `WorkerSource`\r\n - [x] Document `addSourceType` -- includes documenting the `create` function and the `self.registerWorkerSource(...)` requirement on `WorkerSource` modules.\r\n - [x] Document \"How to implement a source type\" -- there's probably a bit more to say beyond the `Source`, `WorkerSource`, and `addSourceType`.  Perhaps an example of a 3rd-party source type would suffice. **update:** went with a link to https://github.com/developmentseed/mapbox-gl-topojson in the docs for `GeoJSONWorkerSource`.\r\n - [x] Update docs for ImageSource, VideoSource, GeoJSONSource to reflect that their constructors are no longer publicly available\r\n - [x] Fix any test regressions\r\n - [x] Add new unit tests as appropriate\r\n - [x] Cleanup - drop async loading of core source types, and do it in the \"right\" place.\r\n - [x] Cleanup - geojson is a core source type, and so I think we should hard-code require its worker source implementation directly in worker.js--in addition to avoiding the function (self) {} wrapper, it would be silly to webworkify it and waste time at runtime sending that code to all the worker threads\r\n - [x] Cleanup - move `loadTileJSON`, `queryRenderedFeatures`, and `querySourceFeatures` out of `source.js`\r\n - [x] Cleanup - fix [upstream bug in WebWorkify](https://github.com/substack/webworkify/issues/26)\r\n - [x] Cleanup - hunt & remove all `TODO`\r\n"},"comment":{"url":"https://api.github.com/repos/mapbox/mapbox-gl-js/issues/comments/230024807","html_url":"https://github.com/mapbox/mapbox-gl-js/pull/2667#issuecomment-230024807","issue_url":"https://api.github.com/repos/mapbox/mapbox-gl-js/issues/2667","id":230024807,"user":{"login":"anandthakker","id":4974151,"avatar_url":"https://avatars.githubusercontent.com/u/4974151?v=3","gravatar_id":"","url":"https://api.github.com/users/anandthakker","html_url":"https://github.com/anandthakker","followers_url":"https://api.github.com/users/anandthakker/followers","following_url":"https://api.github.com/users/anandthakker/following{/other_user}","gists_url":"https://api.github.com/users/anandthakker/gists{/gist_id}","starred_url":"https://api.github.com/users/anandthakker/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/anandthakker/subscriptions","organizations_url":"https://api.github.com/users/anandthakker/orgs","repos_url":"https://api.github.com/users/anandthakker/repos","events_url":"https://api.github.com/users/anandthakker/events{/privacy}","received_events_url":"https://api.github.com/users/anandthakker/received_events","type":"User","site_admin":false},"created_at":"2016-07-01T19:04:32Z","updated_at":"2016-07-01T19:04:32Z","body":"rebased, previous at 73a35079c9537dcdc985683e5262ac15309173b3"}}
{"actor":{"display_login":"anandthakker"}}