Add RFC: Imperative API Improvements (#2621)

dev-docs/RFCs/README.md

@@ -51,6 +51,8 @@ Possible other animation related RFCs:
 
 Group of related RFCs loosely targeted for 7.x releases.
 
+| RFC | Author | Status | Description |
+| --- | ---    | ---    | ---         |
 | [**Binary Data**](v7.x/binary-data-rfc.md) | @ibgreen | **Draft** | Supporting binary data as input to deck.gl layers. |
 
 
@@ -66,7 +68,6 @@ RFCs loosely targeted for 7.x releases. Also see [luma.gl RFCs](https://github.c
 | [**Layer Operations**](v7.x/layer-and-group-operation-rfc.md) | @ibgreen| **Preliminary Approval** | Allow partial updates of vertex attributes to support high-performance editing. |
 | [**Property Animation**](v7.x/property-animation-rfc.md) | @ibgreen | Draft | Allow Layer props and GL parameters to accept functions in addition to values and call these on every render to update values |
 
-
 ## v7.0 RFCs
 
 These RFCS are currently targeted for v7.0. Also see [luma.gl RFCs](https://github.com/uber/luma.gl/tree/master/dev-docs/RFCs#v70-rfcs).

dev-docs/RFCs/v7.x/imperative-api-rfc.md

@@ -0,0 +1,215 @@
+RFC: Improved Support for Imperative Style Programming in deck.gl
+
+Abstract: This RFC proposes small API additions (New `Deck.updateLayers/addLayers/removeLayers` methods and alternate `updateTriggers` semantics) intended to simplify use of the deck.gl API in imperative (non-React) applications.
+
+
+## Background
+
+### Problem 1: Partially Updating Layers
+
+Having to repeat all layer props (and worse, all layers) when changing one prop can get quite demanding for imperative applications.
+
+
+```
+// First time we set the props
+fetch().then(data => {
+  data = transformData(data);
+  deck.setProps({
+    layers: new ScatterplotLayer({
+      data,
+      getPosition: getPositionBinned
+    }),
+    ... // Other layers
+  });
+}
+
+// Now we update the props
+button.onClick(() => {
+  deck.setProps({
+    layers: new ScatterplotLayer({
+      getPosition: getPositionByState, // I just want to change the accessor, but
+      data // I have to repeat all props I already set, but they are out of scope now :(
+    }),
+    // Let's not talk about having to repeat all the other layers I might have created :( :(
+  }
+});
+```
+
+Naturally this can be handled e.g. by creating a global `getLayers` function but essentially it forces a "global" state model onto the app which is usually not the way smaller imperative apps are structured (once set, state is owned by components).
+
+
+### Problem 2: The imperative awkwardness of `updateTriggers`
+
+As the following example shows, having to specify an `updateTrigger` when imperatively saying "I am changing this", just seems excessively verbose and it is not exactly intutive to a new programmer that it would even be needed:
+
+```
+// First time we set the props
+deck.setProps({
+  layers: new ScatterplotLayer({
+    data
+    getPosition: getPositionBinned
+  })
+})
+
+// Now we update the props
+button.onClick(() => {
+  deck.setProps({
+    layers: new ScatterplotLayer({
+      data
+      getPosition: getPositionByState,
+      updateTriggers: {
+        getPosition: getPositionByState // Duh, I just told you two lines ago...
+      }
+    })
+  })
+})
+```
+
+Here the `updateTrigger` just seems to duplicate the programmer's intent. He/she would most likely not have explicitly taken the trouble of updating the `getPosition` accessor unless he intended to change the visualization.
+
+The `updateTriggers` mechanism is highly tuned to the React use case, where the application keeps rendering the same props over and over again, and deck.gl needs to carefully balance between correctly detecting when props have changed, without spending excessive CPU cycles constantly doing deep comparisons of props.
+
+In the imperative use case, the very act of setting new props means that something has probably changed. While deck.gl's "diffing" to avoid unnecessary updates in this case could probably be appreciated by an imperative programmer in certain cases, it is likely not something he/she necessarily expects nor would miss if it wasn't there.
+
+
+## API Design Constraints
+
+* ONE API Principle - See [API Evolution Roadmap](dev-docs/roadmaps/api-evolution-roadmap.md)
+
+* **Layers are Immutable** -  The user would surely like to call `layer.setProps({...})` to partially update a layer but this is not possible in our current API model where layers are immutable descriptors. Instead of exploring the complexities of changing that model, a separate mechanism for updating layers is proposed.
+
+
+## Proposals
+
+This RFC proposes an `updateTriggers` overload (to set alternate semanatics) and new `Deck` method(s) (to partially update layers).
+
+
+### Proposal: `Layer.updateTriggers: true`
+
+In this propsal, we add the option of setting `updateTriggers` to `true` (pre-proposal, `updateTriggers` is expected to be an object). This way an imperative application has an easy way to force accessors to be value compared.
+
+In addition, this could be the default behavior when calling `Deck.updateLayers` as that function is only available in imperative programs.
+
+
+### Deck.updateLayers(layerMap : Object)
+
+Updates specified props in existing layers, without changing any other props. Effectively clones the existing layer, overwriting the specified props.
+
+```
+deck.updateLayers({
+  'scatterplot': {
+    ...newProps
+  }
+});
+```
+
+The `updateLayers` function takes a map with layer id keys, where each value is a map of props to update for that layer.
+
+* UpdateLayers can't remove existing layers
+
+Remarks:
+
+* If no corresponding layer exists, that update key will be ignored (a console warning may be emitted).
+* Updating child layers is currently not supported - this will create state inconsistencies and should happen through a common prop forwarding scheme for sublayers (so that the update goes through the top-level layer).
+* The applicatin's choice of whether to re-generate all props and layers or just update selected props in a few layers is mainly stylistic (deck.gl's highly optimized layer update engine typically makes the performance difference irrelevant).
+
+
+### Deck.addLayers(layers)
+
+> deck.updateLayers can't actually add layers...
+
+* `updateLayers` currently only accepts prop objects. We could enable addition of new layers by accepting layer class instances as new layers `id: new ScatterplotLayer(id, ...)` if we found one of these we could just add/overwrite the layer instead of trying to update an existing layer.
+
+
+### Deck.removeLayers(layerIds)
+
+> If we are adding `addLayers`, we might as well add `removeLayers`.
+
+* UpdateLayers could remove layers? - We could define `layer-id`: `null` to mean that layer should be removed.
+
+
+## Impact
+
+The new APIs/semantics proposed in this RFC will (reasonably) only be used in imperative applications. It will cause e.g. React and Pure JS code examples to look increasingly different, but this is probably justifiable given the big usability wins for the imperative use case.
+
+And we already support partial prop updates of `Deck` props so why not for layer props... We may want to update our "ONE API" design rule to accommodate this.
+
+
+## Concerns
+
+This proposal does generate some concerns (but is put forward based on an assessment that the advantages outweigh the concerns)
+
+* **React API confusion** - One concern that is based on interactions with multiple users is that "inexperienced" React programmers that are uncertain about the performance characteristics of deck.gl always generating all layers will try to use these methods to "optimize" their apps, defeating the "reactive" design of the deck.gl API. The docs should make it clear that this is not recommended.
+
+* **API/example divergence** - Our functional/imperative examples will diverge further, making it harder to easily port an example from one "world" to the "other".
+
+---
+
+## Appendix: Rejected Alternatives
+
+The API can be designed in a number of ways, the following alternatives were considered and rejected.
+
+
+### Single `deck.update` method
+
+More props might benefit from "similar partial" update support as `layers`, e.g. `effects`. Perhaps `deck.update` would be a more extensible API.
+
+```
+deck.update({
+  layers: {
+    'scatterplot': {
+      ...newProps
+    }
+  },
+  effects: {
+    'lighting': {
+      ...newProps
+    }
+  }
+});
+```
+
+Rejected: A downside of this API is that we'd need to list which props we support. Or if this function supported most props, it could become a shadow API to parallel `deck.setProps`, which seems undesirable (e.g ONE API concerns). It also struggles with the add/remove layer cases...
+
+
+### Update layers via a prop instead of a function?
+
+```
+deck.setProps({
+  updateLayers: {
+    'scatterplot': {
+      ...newProps
+    }
+  }
+});
+```
+
+Rejected: A weakness of the `updateLayers` prop as proposed above is that it is not really a proper "prop", e.g. in the sense that its effects are very state dependent, and it would be confusing to use in React applications.
+
+
+### Update using actual layer instances rather than prop objects
+
+```
+deck.updateLayers([
+  new ScatterplotLayer({id: ..., ...newProps})
+});
+```
+
+We still want to only partially update props, so in this case, harvest the specified prop from the new layer instance and only update those using same method as above.
+
+Rejected: Seems like it could be confusing to programmers, providing the full layer but only updating some things.
+
+
+### Modify layer instances (deck.getLayer)
+
+Again, work through actual layer instances instead of descriptors
+
+```
+deck.updateLayers([
+  deck.getLayer('scatterplot-id').clone({
+    ...newProps
+  })
+])
+```
+
+Rejected: looks clunky, requires double layer lookup?

dev-docs/roadmaps/api-evolution-roadmap.md

@@ -0,0 +1,92 @@
+# API Evolution Roadmap
+
+The deck.gl core API will need to be modernized over time. Changes to the core API have a big impact, this roadmap ensures that we have a clear long-term goal to work towards.
+
+
+## Core Principles
+
+### ONE API Principle
+
+deck.gl is designed based on a "ONE API" principle, which is intended as a strong encouragement to API designers to ensure that we do not create irreconcilable forks in our API, as we continue to improve support for the React, Pure JS, scripting, JSON etc versions of the deck.gl API. Ideally a program should be portable between versions with very small differences, and learning how to use deck.gl in one version should translate directly to all versions.
+
+Therefore any proposals here need to consider this principle and justify the design in case of deviations. Of course, by taking the conversation one step up, and talking about imperative vs. reactive/functional paradigms we can argue that this support does not cater to one specific API version but rather to ensure that we support multiple programming paradigms. Still, these changes will make programs written in the various deck.gl API incarnations more different, so it is good to exercise constraint when possible.
+
+
+## Basic default tooltip support
+
+Built in tooltip implementation, so that simple apps don't have to start manipulating DOM / HTML for this basic use case.
+Ideally the default tooltip should somewhat customizable declaratively (without callbacks) for immediate use in JSON layers etc.
+
+
+## Color API
+
+### String valued colors
+
+Support color definitions of type `#abc` and `#aabbcc`, possibly others.
+
+```
+  new ScatterplotLayer({
+  	highlightColor: '#ffee00',
+  	getFillColor: '#aa000'
+  })
+```
+
+Feature has been requested, and we already have a very efficient string to color array parser (`yarn bench` to test), and a prop type system that lets us know what props are colors . We could easily apply this color string parser to color props and accessor return values.
+
+Open questions:
+- What string syntaxes to support (performance vs alignment with CSS, SVG etc).
+- Prop type system extension: accessor return value types also need to be specified in prop types so we can apply this at the right time?
+- Possible conflict with overloading string-valued props for other use cases. I.e. we currently still have the freedom to define the semantics of strings returned from accessors (since we don't currently support it), this would limit that.
+
+
+### Floating Point Colors (0 to 1 instead of 0 to 255)
+
+Background: Exposing colors as [0-255] is somewhat clunky/arbitrary. Many applications and frameworks use floats `0`-`1` to represent components. Misunderstandings (passing in a float) sometimes lead to nothing rendering and lost developer time.
+
+Some thoughts:
+- Need to analyze breaking changes.
+- Use attribute normalization flag, avoid manually dividing colors with 255 in shaders. Would let us mix integer and float attributes.
+- Can we supporting both float and int colors? Autodetection (component > 1 ? 'int' : 'float') is almost but not quite practical. Letting user provide hints: `Float32Array` vs `Uint32Array` input would make things clear.
+
+
+## Imperative Programming API Improvements
+
+deck.gl supports both functional (react, json) and imperative (pure-js, scripting) programming paradigms. The original design was laser focused on making sure that the library is optimized "to the bone" for the react/functional programming paradigm. This has lead to an API that is slightly surprising/clunky to use for imperative style programs. This section explores smaller changes that can be done to facilitate imperative programming without compromising performance/support for react/functional programming.
+
+
+### Proposal: Support partially updating layer props (v7.0 API)
+
+Reference: [Imperative Programming RFC](/dev-docs/RFCs/v7.x/imperative-programming-rfc.md).
+
+Summary: One could say that React "sets all the props of all the layers all the time". This is how the deck.gl API is designed (the `layers` prop needs to be completely resupplied whenever any prop in any layer changes). Imperative programs are different, they normally just want to change a few props in one layer.
+
+
+### Proposal: Enforce updateTriggers (v7.0 API)
+
+Reference: [Imperative Programming RFC]().
+
+Summary: React apps often set accessors again and again, to new copies of identical functions (generated by using "inline" functions). To avoid performance issues, deck.gl provides the `updateTriggers` mechanism. Imperative programs are different, `updateTriggers` don't make much sense.
+
+
+### Proposal: Proper `LayerState` class hierarchy (v7.0 API)
+
+Reference: 7.0 API
+
+> This is a partially related (imperative API improvements) idea that would mostly affect/benefit layer writers, rather than layer users
+
+Something that tends to confuse imperative programmers is that the Layer class is a transient descriptor. In React, the `React.Component` subclass (the most direct equicalent to the deck.gl `Layer` sublass) is the permanent entity, while the transient descriptors are the React `Elements` (many React users do not even realize that these `Elements` are created as this happens transparently by transpiled JSX).
+
+One way to move closer to this model would be to move towards a proper LayerState class/sublass setup.
+
+class LineLayerState extends LayerState {
+  ...
+}
+
+This would let layer subclass writers focus on the permanent part of the Layer, as opposed to putting all their logic on the "dumb" descriptor.
+
+A number of Layer methods should probably move to the Layer state, and be forwarded for now for backwards compatibility.
+
+Note that this would not break the React JSX style of specifying the layer props?
+`<LineLayer data={...} {...otherProps}/>` since those methods are not used directly in this syntax.
+
+More thought would also need to go into how applications could benefit from this change.