[Chore]: Technical: constants and types modules isolation #1840

dariaterekhova-actionengine · 2022-06-03T18:39:34Z

No description provided.

jagerts · 2022-06-06T10:35:28Z

src/components/side-panel/layer-panel/add-layer-button.tsx

@@ -29,7 +29,8 @@ import {Button} from 'components/common/styled-components';
 import {DatasetSquare} from 'components';
 import Typeahead from 'components/common/item-selector/typeahead';
 import Accessor from 'components/common/item-selector/accessor';
-import {Datasets, RGBColor} from 'reducers';
+import {Datasets} from 'reducers';


Shouldn't it been moved to @kepler.gl/types as well? Datasets is used in different modules - utils, components, etc.

May be we should move all types there..? @heshan0131 what do you think about it?

Let's do it with utils isolation

yea move it to types would be ideal

jagerts · 2022-06-06T10:37:52Z

src/components/side-panel/layer-panel/color-selector.tsx

@@ -27,8 +27,8 @@ import ColorPalette from './color-palette';
 import {StyledPanelDropdown} from 'components/common/styled-components';
 import onClickOutside from 'react-onclickoutside';
 import {ColorUI} from 'layers/layer-factory';
-import {ColorRange} from 'constants/color-ranges';
-import {NestedPartial, RGBColor} from 'reducers';
+import {ColorRange} from '@kepler.gl/constants/color-ranges';


It should be only '@kepler.gl/constants' here?

No, I added export of all other paths, so we now can keep imports like this and save some time on future isolations

better to import it from @kepler.gl/constants so when we are building kepler from src in studio, we don't need to add additional path resolver to our webpack config

jagerts · 2022-06-06T11:06:34Z

src/constants/index.ts

@@ -1,77 +0,0 @@
-// Copyright (c) 2022 Uber Technologies, Inc.


Looks like this file should be moved to ./src, but it is deleted

It has a lot of changes in it, so git decided that index.ts in src folder is taken form src/layers/types.ts

jagerts · 2022-06-06T11:09:11Z

src/constants/src/index.ts

@@ -18,20 +18,16 @@
 // OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
 // THE SOFTWARE.

-import keyMirror from 'keymirror';


And this file should be deleted?

See previous comment

heshan0131 · 2022-06-08T15:59:21Z

src/actions/actions.ts

@@ -18,11 +18,11 @@
 // OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
 // THE SOFTWARE.

-import ActionTypes from 'constants/action-types';
+import {ActionTypes} from '@kepler.gl/constants';


is it possible to move this to actions? or define as emun and move it to types?

heshan0131 · 2022-06-08T16:00:37Z

src/actions/provider-actions.ts

@@ -19,7 +19,7 @@
 // THE SOFTWARE.

 import {createAction} from '@reduxjs/toolkit';
-import {ACTION_PREFIX} from 'constants/action-types';
+import {ACTION_PREFIX} from '@kepler.gl/constants';


move it to actions?

heshan0131 · 2022-06-08T16:03:02Z

src/components/editor/editor.tsx


 import {DEFAULT_RADIUS, getStyle as getFeatureStyle} from './feature-styles';
 import {getStyle as getEditHandleStyle, getEditHandleShape} from './handle-style';
-import KeyEvent from 'constants/keyevent';
+import {KeyEvent} from '@kepler.gl/constants';


combine these 2 imports from '@kepler.gl/constants';

heshan0131 · 2022-06-08T16:03:17Z

src/components/filters/polygon-filter.tsx

@@ -21,7 +21,7 @@
 import React, {useMemo, useCallback} from 'react';
 import ItemSelector from 'components/common/item-selector/item-selector';
 import {Layer} from 'layers';
-import {LAYER_TYPES} from 'layers/types';
+import {LAYER_TYPES} from '@kepler.gl/constants';


ideally move this to layers

heshan0131 · 2022-06-08T16:16:32Z

src/components/filters/polygon-filter.tsx

-        filter,
-        layers
-      ]);
+  const PolygonFilter: React.FC<PolygonFilterProps> = React.memo(({filter, layers, setLayers}) => {


why are these styling changed? is there a prettier version upgrade?

No, I haven't upgraded prettier or it's configuration... But I've checked now and prettier has changed noting so should I return the previous format there manually?

heshan0131 · 2022-06-08T16:21:47Z

src/components/side-panel/layer-panel/custom-palette.tsx

@@ -29,8 +29,8 @@ import {VertDots, Trash} from 'components/common/icons';
 import ColorPalette from './color-palette';
 import CustomPicker from './custom-picker';
 import {arrayMove} from 'utils/data-utils';
-import {ColorRange} from 'constants/color-ranges';
-import {NestedPartial} from 'reducers';
+import {ColorRange} from '@kepler.gl/constants/color-ranges';


heshan0131 · 2022-06-08T16:35:58Z

src/constants/babel.config.js

+const PLUGINS = [
+  ['@babel/plugin-transform-typescript', {isTSX: true, allowDeclareFields: true}],
+  '@babel/plugin-transform-modules-commonjs',
+  '@babel/plugin-proposal-class-properties',


do you need this for constants? are we exporting and class from constant?

heshan0131 · 2022-06-08T16:36:38Z

src/constants/package.json

+    "build:types": "tsc --project ./tsconfig.production.json",
+    "prepublish": "uber-licence && yarn build && yarn build:umd && yarn build:types"
+  },
+  "files": [


you probably don't need any of this here

heshan0131 · 2022-06-08T16:39:37Z

src/constants/package.json

+  "nyc": {
+    "sourceMap": false,
+    "instrument": false,
+    "exclude": [


no need for these excludes

heshan0131 · 2022-06-08T16:39:49Z

src/constants/package.json

+    "webpack-stats-plugin": "^0.2.1"
+  },
+  "peerDependencies": {
+    "react": ">=16.3",


don't need these peerDependencies

heshan0131 · 2022-06-08T16:41:06Z

src/constants/package.json

+    "typescript": "4.2.3",
+    "uber-licence": "^3.1.1",
+    "webpack": "^4.29.0",
+    "webpack-bundle-analyzer": "^3.3.2",


don't need these webpack plugins

heshan0131 · 2022-06-08T16:42:31Z

src/constants/package.json

+  "name": "@kepler.gl/constants",
+  "author": "Shan He <shan@uber.com>",
+  "version": "2.5.5",
+  "description": "kepler.gl is a webgl based application to visualize large scale location data in the browser",


edit description:
kepler.gl constants used by kepler.gl components, actions and reducers

Signed-off-by: Daria Terekhova <daria.terekhova@actionengine.com>

heshan0131 · 2022-06-08T16:44:18Z

src/constants/src/action-types.ts

@@ -183,6 +183,6 @@ const ActionTypes = {
  START_SAVE_STORAGE: `${ACTION_PREFIX}START_SAVE_STORAGE`
 };

-const assignType = <T>(obj: T): { [K in keyof T]: `${typeof ACTION_PREFIX}${string & K}`; } => obj as any
+const assignType = <T>(obj: T): { [K in keyof T]: `${typeof ACTION_PREFIX}${string & K}`; } => obj as any // eslint-disable-line


preferably move this to top of this line

Signed-off-by: Daria Terekhova <daria.terekhova@actionengine.com>

dariaterekhova-actionengine force-pushed the dt/isolation_base branch from 72bef9c to bb00809 Compare June 3, 2022 18:40

dariaterekhova-actionengine requested review from heshan0131, jagerts and HeimEndyd June 3, 2022 19:00

jagerts reviewed Jun 6, 2022

View reviewed changes

heshan0131 reviewed Jun 8, 2022

View reviewed changes

dariaterekhova-actionengine added 10 commits June 9, 2022 14:48

Moved constants folder content to constants/src

2ff1e52

Signed-off-by: Daria Terekhova <daria.terekhova@actionengine.com>

Import updates

981a66f

Signed-off-by: Daria Terekhova <daria.terekhova@actionengine.com>

Types package

278601a

Signed-off-by: Daria Terekhova <daria.terekhova@actionengine.com>

Export updates

d344cdb

Signed-off-by: Daria Terekhova <daria.terekhova@actionengine.com>

Rebase corrections

dd7353c

Signed-off-by: Daria Terekhova <daria.terekhova@actionengine.com>

Configuration update

de2d38b

Signed-off-by: Daria Terekhova <daria.terekhova@actionengine.com>

Prettier upgrade

72f3623

Signed-off-by: Daria Terekhova <daria.terekhova@actionengine.com>

Eslint configuration changes

da2f6b2

Signed-off-by: Daria Terekhova <daria.terekhova@actionengine.com>

import corrections

33dc6be

Signed-off-by: Daria Terekhova <daria.terekhova@actionengine.com>

Getting rid of internal imports

98e40e2

Signed-off-by: Daria Terekhova <daria.terekhova@actionengine.com>

dariaterekhova-actionengine added 2 commits June 10, 2022 19:58

LAYER_TYPES moved to layers

c321895

Signed-off-by: Daria Terekhova <daria.terekhova@actionengine.com>

Scripts update

756e1d0

Signed-off-by: Daria Terekhova <daria.terekhova@actionengine.com>

dariaterekhova-actionengine force-pushed the dt/isolation_base branch from bb00809 to 756e1d0 Compare June 10, 2022 16:59

ActionTypes moved to actions

b474a2e

Signed-off-by: Daria Terekhova <daria.terekhova@actionengine.com>

heshan0131 approved these changes Jun 14, 2022

View reviewed changes

Review correction

6798081

Signed-off-by: Daria Terekhova <daria.terekhova@actionengine.com>

dariaterekhova-actionengine merged commit 10868ec into master Jun 14, 2022

delete-merged-branch bot deleted the dt/isolation_base branch June 14, 2022 10:34

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Chore]: Technical: constants and types modules isolation #1840

[Chore]: Technical: constants and types modules isolation #1840

dariaterekhova-actionengine commented Jun 3, 2022

jagerts Jun 6, 2022 •

edited

dariaterekhova-actionengine Jun 6, 2022

heshan0131 Jun 8, 2022

jagerts Jun 6, 2022

dariaterekhova-actionengine Jun 6, 2022

heshan0131 Jun 8, 2022

jagerts Jun 6, 2022

dariaterekhova-actionengine Jun 6, 2022

jagerts Jun 6, 2022

dariaterekhova-actionengine Jun 6, 2022

heshan0131 Jun 8, 2022 •

edited

heshan0131 Jun 8, 2022

heshan0131 Jun 8, 2022

heshan0131 Jun 8, 2022 •

edited

heshan0131 Jun 8, 2022

dariaterekhova-actionengine Jun 9, 2022

heshan0131 Jun 8, 2022

heshan0131 Jun 8, 2022

heshan0131 Jun 8, 2022

heshan0131 Jun 8, 2022

heshan0131 Jun 8, 2022

heshan0131 Jun 8, 2022

heshan0131 Jun 8, 2022

heshan0131 Jun 8, 2022

		@@ -1,77 +0,0 @@
		// Copyright (c) 2022 Uber Technologies, Inc.

[Chore]: Technical: constants and types modules isolation #1840

[Chore]: Technical: constants and types modules isolation #1840

Conversation

dariaterekhova-actionengine commented Jun 3, 2022

jagerts Jun 6, 2022 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

heshan0131 Jun 8, 2022 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

heshan0131 Jun 8, 2022 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jagerts Jun 6, 2022 •

edited

heshan0131 Jun 8, 2022 •

edited

heshan0131 Jun 8, 2022 •

edited