diff --git a/modules/json/src/helpers/convert-functions.js b/modules/json/src/helpers/convert-functions.js index 72496ef04b3..a98a4b5b21e 100644 --- a/modules/json/src/helpers/convert-functions.js +++ b/modules/json/src/helpers/convert-functions.js @@ -1,27 +1,30 @@ import parseExpressionString from './parse-expression-string'; -import {getPropTypes, isFunctionProp} from './deck-prop-types'; -// Try to determine if any props are function valued -// and if so convert their string values to functions -export default function convertFunctions(Class, props, configuration) { - const propTypes = getPropTypes(Class); +import {FUNCTION_IDENTIFIER} from '../syntactic-sugar'; - // Use deck.gl prop types if available. - return convertFunctionsUsingPropTypes(props, propTypes, configuration); +function hasFunctionIdentifier(value) { + return typeof value === 'string' && value.startsWith(FUNCTION_IDENTIFIER); +} + +function trimFunctionIdentifier(value) { + return value.replace(FUNCTION_IDENTIFIER, ''); } -function convertFunctionsUsingPropTypes(props, propTypes, configuration) { +// Try to determine if any props are function valued +// and if so convert their string values to functions +export default function convertFunctions(props, configuration) { + // Use deck.gl prop types if available. const replacedProps = {}; for (const propName in props) { let propValue = props[propName]; // Parse string valued expressions - const isFunction = isFunctionProp(propTypes, propName); + const isFunction = hasFunctionIdentifier(propValue); - // Parse string as "expression", return equivalent JavaScript function - if (isFunction && typeof propValue === 'string') { - const isAccessor = true; - propValue = parseExpressionString(propValue, configuration, isAccessor); + if (isFunction) { + // Parse string as "expression", return equivalent JavaScript function + propValue = trimFunctionIdentifier(propValue); + propValue = parseExpressionString(propValue, configuration); } // Invalid functions return null, show default value instead. diff --git a/modules/json/src/helpers/deck-prop-types.js b/modules/json/src/helpers/deck-prop-types.js deleted file mode 100644 index e4945ed1c17..00000000000 --- a/modules/json/src/helpers/deck-prop-types.js +++ /dev/null @@ -1,31 +0,0 @@ -// NOTE - This is sniffing for deck.gl prop types -// TODO - We may want to do something similar for React `prop-types` -// TODO - We need something similar for non-deck, non-React classes -// TODO - The deck prop types system could potentially be exported as a general prop types package.... - -export function getPropTypes(Class) { - let propTypes = Class && Class._propTypes && Class._propTypes; - // HACK: Trigger generation of propTypes - if (!propTypes && Class && Class.defaultProps) { - new Class({}); // eslint-disable-line no-new - propTypes = Class && Class._propTypes && Class._propTypes; - } - return propTypes; -} - -export function isFunctionProp(propTypes, propName) { - const propType = propTypes && propTypes[propName]; - if (!propType) { - // TODO - simple heuristic if prop types are not avaialable - return propName.startsWith('get'); - } - - const type = typeof propType === 'object' && propType.type; - switch (type) { - case 'accessor': - case 'function': - return true; - default: - return false; - } -} diff --git a/modules/json/src/helpers/instantiate-class.js b/modules/json/src/helpers/instantiate-class.js index c82a728d808..464a09fcd84 100644 --- a/modules/json/src/helpers/instantiate-class.js +++ b/modules/json/src/helpers/instantiate-class.js @@ -27,9 +27,7 @@ function instantiateJavaScriptClass(Class, props, configuration) { if (configuration.preProcessClassProps) { props = configuration.preProcessClassProps(Class, props, configuration); } - - props = convertFunctions(Class, props, configuration); - + props = convertFunctions(props, configuration); return new Class(props); } @@ -41,7 +39,7 @@ function instantiateReactComponent(Component, props, configuration) { props = configuration.preProcessClassProps(Component, props, configuration); } - props = convertFunctions(Component, props, configuration); + props = convertFunctions(props, configuration); return React.createElement(Component, props, children); } diff --git a/modules/json/src/json-configuration.js b/modules/json/src/json-configuration.js index 05a4655b345..2e98524a783 100644 --- a/modules/json/src/json-configuration.js +++ b/modules/json/src/json-configuration.js @@ -2,14 +2,14 @@ import parseExpressionString from './helpers/parse-expression-string'; import assert from './utils/assert'; -const DEFAULT_TYPE_KEY = '@@type'; +import {TYPE_KEY} from './syntactic-sugar'; const isObject = value => value && typeof value === 'object'; export default class JSONConfiguration { constructor(...configurations) { // Initialize config with default values - this.typeKey = DEFAULT_TYPE_KEY; + this.typeKey = TYPE_KEY; this.log = console; // eslint-disable-line this.classes = {}; this.reactComponents = {}; @@ -50,6 +50,6 @@ export default class JSONConfiguration { } } -function convertFunction(value, key, configuration) { +function convertFunction(value, configuration) { return parseExpressionString(value, configuration); } diff --git a/modules/json/src/json-converter.js b/modules/json/src/json-converter.js index 1c085e5403a..7d12107ebf7 100644 --- a/modules/json/src/json-converter.js +++ b/modules/json/src/json-converter.js @@ -11,11 +11,11 @@ import assert from './utils/assert'; import JSONConfiguration from './json-configuration'; import {instantiateClass} from './helpers/instantiate-class'; + +import {FUNCTION_IDENTIFIER, CONSTANT_IDENTIFIER} from './syntactic-sugar'; import parseJSON from './helpers/parse-json'; const isObject = value => value && typeof value === 'object'; -const FUNCTION_IDENTIFIER = '@@='; -const CONSTANT_IDENTIFIER = '@@#'; export default class JSONConverter { constructor(props) { @@ -82,7 +82,7 @@ function convertJSONRecursively(json, key, configuration) { return json.map((element, i) => convertJSONRecursively(element, String(i), configuration)); } - // If object.type is in configuration, instantitate + // If object.type is in configuration, instantiate if (isClassInstance(json, configuration)) { return convertClassInstance(json, configuration); } @@ -103,7 +103,8 @@ function convertJSONRecursively(json, key, configuration) { // Returns true if an object has a `type` field function isClassInstance(json, configuration) { const {typeKey} = configuration; - return isObject(json) && Boolean(json[typeKey]); + const isClass = isObject(json) && Boolean(json[typeKey]); + return isClass; } function convertClassInstance(json, configuration) { @@ -139,7 +140,7 @@ function convertString(string, key, configuration) { // Here the JSON value is supposed to be treated as a function if (string.startsWith(FUNCTION_IDENTIFIER) && configuration.convertFunction) { string = string.replace(FUNCTION_IDENTIFIER, ''); - return configuration.convertFunction(string, key, configuration); + return configuration.convertFunction(string, configuration); } if (string.startsWith(CONSTANT_IDENTIFIER)) { string = string.replace(CONSTANT_IDENTIFIER, ''); diff --git a/modules/json/src/syntactic-sugar.js b/modules/json/src/syntactic-sugar.js new file mode 100644 index 00000000000..3a6fe67d5c5 --- /dev/null +++ b/modules/json/src/syntactic-sugar.js @@ -0,0 +1,5 @@ +const FUNCTION_IDENTIFIER = '@@='; +const CONSTANT_IDENTIFIER = '@@#'; +const TYPE_KEY = '@@type'; + +export {FUNCTION_IDENTIFIER, CONSTANT_IDENTIFIER, TYPE_KEY}; diff --git a/test/modules/json/data/deck-props.json b/test/modules/json/data/deck-props.json index f7cd0d2c611..7bc63523d69 100644 --- a/test/modules/json/data/deck-props.json +++ b/test/modules/json/data/deck-props.json @@ -51,7 +51,7 @@ 0, 3000 ], - "getText": "Hello World", + "getText": "@@=Hello World", "billboard": false } ] diff --git a/test/modules/json/helpers/convert-functions.spec.js b/test/modules/json/helpers/convert-functions.spec.js index bb2cb8f70d4..21a893d379e 100644 --- a/test/modules/json/helpers/convert-functions.spec.js +++ b/test/modules/json/helpers/convert-functions.spec.js @@ -75,27 +75,30 @@ const TEST_CASES = [ {expr: 'this.three', expected: 3} ]; -test('convertFunctions#get...', t => { +test('convertFunctions#asStrings', t => { const props = {}; TEST_CASES.forEach((testCase, i) => { // Add a mix of function and value props - // TODO this is based on `get` heuristic, we have better ideas and will update tests when we address - const propName = i % 2 === 0 ? `get-${i}` : `value-${i}`; - props[propName] = testCase.expr; + props[`value-{i}`] = testCase.expr; }); - const convertedProps = convertFunctions(null, props, {}); + const convertedProps = convertFunctions(props, {}); for (const key in convertedProps) { - if (key.startsWith('get')) { - t.ok( - typeof convertedProps[key] === 'function', - 'convertFunctions converted prop to function' - ); - } else { - t.ok( - typeof convertedProps[key] !== 'function', - 'convertFunctions did not converted string to function' - ); - } + t.ok( + typeof convertedProps[key] !== 'function', + 'convertFunctions did not convert string to function' + ); + } + t.end(); +}); + +test('convertFunctions#asFunctions', t => { + const props = {}; + TEST_CASES.forEach((testCase, i) => { + props[`func-{i}`] = `@@=${testCase.expr}`; + }); + const convertedProps = convertFunctions(props, {}); + for (const key in convertedProps) { + t.ok(typeof convertedProps[key] === 'function', 'convertFunctions converted prop to function'); } t.end(); }); diff --git a/test/modules/json/helpers/deck-prop-types.spec.js b/test/modules/json/helpers/deck-prop-types.spec.js deleted file mode 100644 index 38c33f17c17..00000000000 --- a/test/modules/json/helpers/deck-prop-types.spec.js +++ /dev/null @@ -1,12 +0,0 @@ -import test from 'tape-catch'; -import {getPropTypes, isFunctionProp} from '@deck.gl/json/helpers/deck-prop-types'; - -import {ScatterplotLayer} from '@deck.gl/layers'; - -test('json#getPropTypes', t => { - const propTypes = getPropTypes(ScatterplotLayer); - t.ok(propTypes, 'Got prop types from deck.gl ScatterplotLayer'); - t.ok(isFunctionProp(propTypes, 'getFillColor'), 'ScatterplotLayer.getFillColor is a function'); - t.notOk(isFunctionProp(propTypes, 'sizeScale'), 'ScatterplotLayer.sizeScale is not function'); - t.end(); -}); diff --git a/test/modules/json/index.js b/test/modules/json/index.js index b67d7879e93..b8d185e8e86 100644 --- a/test/modules/json/index.js +++ b/test/modules/json/index.js @@ -1,7 +1,6 @@ import './utils/get.spec'; import './utils/shallow-equal-objects.spec'; -import './helpers/deck-prop-types.spec'; import './helpers/parse-expression-string.spec'; import './helpers/convert-functions.spec'; diff --git a/test/modules/json/json-render.spec.js b/test/modules/json/json-render.spec.js index 3c5c989777e..6c673b79564 100644 --- a/test/modules/json/json-render.spec.js +++ b/test/modules/json/json-render.spec.js @@ -11,7 +11,6 @@ test('JSONConverter#render', t => { const deckProps = jsonConverter.convert(JSON_DATA); t.ok(deckProps, 'JSONConverter converted correctly'); - const jsonDeck = new Deck( Object.assign( { diff --git a/test/render/golden-images/jupyter-widget-failed-function.png b/test/render/golden-images/jupyter-widget-failed-function.png new file mode 100644 index 00000000000..760937df930 Binary files /dev/null and b/test/render/golden-images/jupyter-widget-failed-function.png differ diff --git a/test/render/golden-images/jupyter-widget-hexagon-layer-function-syntax.png b/test/render/golden-images/jupyter-widget-hexagon-layer-function-syntax.png new file mode 100644 index 00000000000..107781b3cca Binary files /dev/null and b/test/render/golden-images/jupyter-widget-hexagon-layer-function-syntax.png differ diff --git a/test/render/jupyter-widget.js b/test/render/jupyter-widget.js index 595a5ec9609..fcf4edce97d 100644 --- a/test/render/jupyter-widget.js +++ b/test/render/jupyter-widget.js @@ -17,7 +17,7 @@ const TEST_CASES = [ { '@@type': 'ScatterplotLayer', data: [[0, 0], [0.01, 0.01]], - getPosition: '-', + getPosition: '@@=-', getRadius: 500, getFillColor: [255, 0, 0] } @@ -54,8 +54,8 @@ const TEST_CASES = [ rgb: [123, 159, 53] } ], - getColor: 'rgb', - getPosition: 'position', + getColor: '@@=rgb', + getPosition: '@@=position', getRadius: 100 }, { @@ -72,8 +72,8 @@ const TEST_CASES = [ ], fontSize: 144, getColor: [0, 0, 255], - getPosition: 'position', - getTextAnchor: '"start"', + getPosition: '@@=position', + getTextAnchor: 'start', fontFamily: 'Times, Times New Roman, Georgia, serif' } ] @@ -122,6 +122,183 @@ const TEST_CASES = [ ] }, goldenImage: './test/render/golden-images/jupyter-widget-geojsonlayer.png' + }, + { + name: 'HexagonLayer with Function', + json: { + description: 'HexagonLayer with a function string', + viewState: { + longitude: 0, + latitude: 0, + zoom: 5, + pitch: 40.5, + bearing: -27.396674584323023 + }, + views: [ + { + '@@type': 'MapView', + controller: true + } + ], + layers: [ + { + '@@type': 'HexagonLayer', + id: 'heatmap', + data: [ + {lat: 0, lon: 0}, + {lat: 0, lon: 0}, + {lat: 0, lon: 0}, + {lat: 0, lon: 1}, + {lat: 0.1, lon: 1}, + {lat: 0.1, lon: 0.1}, + {lat: 0.1, lon: 0.1}, + {lat: 0.1, lon: 0.1}, + {lat: 0.1, lon: 0.1}, + {lat: 0.2, lon: 1.2}, + {lat: 0.2, lon: 1.2}, + {lat: 0.2, lon: 1.2}, + {lat: 0.2, lon: 1.2}, + {lat: 0.1, lon: 0.1}, + {lat: 0.1, lon: 0.1}, + {lat: 1.2, lon: 1.2}, + {lat: 1.2, lon: 1.2}, + {lat: 1.2, lon: 1.2}, + {lat: 0.1, lon: 0.1}, + {lat: 1.2, lon: 1.2}, + {lat: 1.2, lon: 1.2}, + {lat: 0.1, lon: 0.1}, + {lat: 1.2, lon: 1.2}, + {lat: 1.2, lon: 1.2}, + {lat: 1.2, lon: 1.2}, + {lat: 1.2, lon: 1.2} + ], + elevationRange: [0, 150], + elevationScale: 1800, + extruded: true, + getPosition: '@@=[lon, lat]', + radius: 10000, + upperPercentile: 100, + colorRange: [ + [1, 152, 189], + [73, 227, 206], + [216, 254, 181], + [254, 237, 177], + [254, 173, 84], + [209, 55, 78] + ] + } + ] + }, + goldenImage: './test/render/golden-images/jupyter-widget-hexagon-layer-function-syntax.png' + }, + { + name: 'Failed HexagonLayer, Successful Heatmap', + json: { + description: + 'HexagonLayer without a function string should fail but HeatmapLayer should succeed', + viewState: { + longitude: 0, + latitude: 0, + zoom: 5, + pitch: 40.5, + bearing: -27.396674584323023 + }, + views: [ + { + '@@type': 'MapView', + controller: true + } + ], + layers: [ + { + '@@type': 'HexagonLayer', + id: 'failed-heatmap', + data: [ + {lat: 0, lon: 0}, + {lat: 0, lon: 0}, + {lat: 0, lon: 0}, + {lat: 0, lon: 1}, + {lat: 0.1, lon: 1}, + {lat: 0.1, lon: 0.1}, + {lat: 0.1, lon: 0.1}, + {lat: 0.1, lon: 0.1}, + {lat: 0.1, lon: 0.1}, + {lat: 0.2, lon: 1.2}, + {lat: 0.2, lon: 1.2}, + {lat: 0.2, lon: 1.2}, + {lat: 0.2, lon: 1.2}, + {lat: 0.1, lon: 0.1}, + {lat: 0.1, lon: 0.1}, + {lat: 1.2, lon: 1.2}, + {lat: 1.2, lon: 1.2}, + {lat: 1.2, lon: 1.2}, + {lat: 0.1, lon: 0.1}, + {lat: 1.2, lon: 1.2}, + {lat: 1.2, lon: 1.2}, + {lat: 0.1, lon: 0.1}, + {lat: 1.2, lon: 1.2}, + {lat: 1.2, lon: 1.2}, + {lat: 1.2, lon: 1.2}, + {lat: 1.2, lon: 1.2} + ], + elevationRange: [0, 15], + elevationScale: 1800, + getPosition: '[lon, lat]', + radius: 10000, + upperPercentile: 100, + colorRange: [ + [1, 152, 189], + [73, 227, 206], + [216, 254, 181], + [254, 237, 177], + [254, 173, 84], + [209, 55, 78] + ] + }, + { + '@@type': 'HeatmapLayer', + id: 'successful-heatmap', + data: [ + {lat: 0, lon: 0}, + {lat: 0, lon: 0}, + {lat: 0, lon: 0}, + {lat: 0, lon: 1}, + {lat: 0.1, lon: 1}, + {lat: 0.1, lon: 0.1}, + {lat: 0.1, lon: 0.1}, + {lat: 0.1, lon: 0.1}, + {lat: 0.1, lon: 0.1}, + {lat: 0.2, lon: 1.2}, + {lat: 0.2, lon: 1.2}, + {lat: 0.2, lon: 1.2}, + {lat: 0.2, lon: 1.2}, + {lat: 0.1, lon: 0.1}, + {lat: 0.1, lon: 0.1}, + {lat: 1.2, lon: 1.2}, + {lat: 1.2, lon: 1.2}, + {lat: 1.2, lon: 1.2}, + {lat: 0.1, lon: 0.1}, + {lat: 1.2, lon: 1.2}, + {lat: 1.2, lon: 1.2}, + {lat: 0.1, lon: 0.1}, + {lat: 1.2, lon: 1.2}, + {lat: 1.2, lon: 1.2}, + {lat: 1.2, lon: 1.2}, + {lat: 1.2, lon: 1.2} + ], + getPosition: '@@=[lon, lat]', + colorRange: [ + [1, 152, 189], + [73, 227, 206], + [216, 254, 181], + [254, 237, 177], + [254, 173, 84], + [209, 55, 78] + ] + } + ] + }, + goldenImage: './test/render/golden-images/jupyter-widget-failed-function.png' } ]; @@ -152,7 +329,6 @@ test('jupyter-widget Render Test', t => { } }; }); - // TODO can add a done POST here } function nextTest() {