Skip to content

Commit

Permalink
Replace Reselect with Memoize (#2042)
Browse files Browse the repository at this point in the history
## Which problem is this PR solving?
Resolves #2031
## Description of the changes
Remove package Reselect
Delete file selectors/dependencies
Add the memoize function to components/DependencyGraph/index.tsx

## How was this change tested?
yarn lint & yarn test
<img width="514" alt="Screenshot 2023-12-10 at 1 13 35 PM"
src="https://github.com/jaegertracing/jaeger-ui/assets/9301491/17c53891-ed5a-44a3-98ef-48b9a6280e05">

## Checklist
- [x] I have read
https://github.com/jaegertracing/jaeger/blob/master/CONTRIBUTING_GUIDELINES.md
- [x] I have signed all commits
- [x] I have added unit tests for the new functionality
- [x] I have run lint and test steps successfully
  - for `jaeger`: `make lint test`
  - for `jaeger-ui`: `yarn lint` and `yarn test`

---------

Signed-off-by: Muzi Qiu <[email protected]>
  • Loading branch information
cooleditphoto authored Dec 10, 2023
1 parent b76514d commit 4bdb8ac
Show file tree
Hide file tree
Showing 5 changed files with 45 additions and 71 deletions.
1 change: 0 additions & 1 deletion packages/jaeger-ui/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,6 @@
"redux-first-history": "^5.1.1",
"redux-form": "^8.3.10",
"redux-promise-middleware": "^6.1.3",
"reselect": "^4.1.6",
"store": "^2.0.12",
"ts-key-enum": "^2.0.0",
"tween-functions": "^1.2.0",
Expand Down
47 changes: 45 additions & 2 deletions packages/jaeger-ui/src/components/DependencyGraph/index.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import { Tabs } from 'antd';
import PropTypes from 'prop-types';
import { bindActionCreators } from 'redux';
import { connect } from 'react-redux';
import memoizeOne from 'memoize-one';

import DAG from './DAG';
import DependencyForceGraph from './DependencyForceGraph';
Expand All @@ -25,7 +26,6 @@ import LoadingIndicator from '../common/LoadingIndicator';
import * as jaegerApiActions from '../../actions/jaeger-api';
import { FALLBACK_DAG_MAX_NUM_SERVICES } from '../../constants';
import { nodesPropTypes, linksPropTypes } from '../../propTypes/dependencies';
import { formatDependenciesAsNodesAndLinks } from '../../selectors/dependencies';
import { getConfigValue } from '../../utils/config/get-config';

import './index.css';
Expand Down Expand Up @@ -128,13 +128,56 @@ export class DependencyGraphPageImpl extends Component {
}
}

const formatDependenciesAsNodesAndLinks = memoizeOne(dependencies => {
const data = dependencies.reduce(
(response, link) => {
const { nodeMap } = response;
let { links } = response;

// add both the parent and child to the node map, or increment their
// call count.
nodeMap[link.parent] = nodeMap[link.parent] ? nodeMap[link.parent] + link.callCount : link.callCount;
nodeMap[link.child] = nodeMap[link.child]
? response.nodeMap[link.child] + link.callCount
: link.callCount;

// filter out self-dependent
if (link.parent !== link.child) {
links = links.concat([
{
source: link.parent,
target: link.child,
callCount: link.callCount,
value: Math.max(Math.sqrt(link.callCount / 10000), 1),
target_node_size: Math.max(Math.log(nodeMap[link.child] / 1000), 3),
},
]);
}

return { nodeMap, links };
},
{ nodeMap: {}, links: [] }
);

data.nodes = Object.keys(data.nodeMap).map(id => ({
callCount: data.nodeMap[id],
radius: Math.max(Math.log(data.nodeMap[id] / 1000), 3),
orphan: data.links.findIndex(link => id === link.source || id === link.target) === -1,
id,
}));

const { nodes, links } = data;

return { nodes, links };
});

// export for tests
export function mapStateToProps(state) {
const { dependencies, error, loading } = state.dependencies;
let links;
let nodes;
if (dependencies && dependencies.length > 0) {
const formatted = formatDependenciesAsNodesAndLinks({ dependencies });
const formatted = formatDependenciesAsNodesAndLinks(dependencies);
links = formatted.links;
nodes = formatted.nodes;
}
Expand Down
62 changes: 0 additions & 62 deletions packages/jaeger-ui/src/selectors/dependencies.js

This file was deleted.

1 change: 0 additions & 1 deletion packages/jaeger-ui/tsconfig.lint.json
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,6 @@
"src/reducers/index.js",
"src/reducers/services.js",
"src/reducers/trace.js",
"src/selectors/dependencies.js",
"src/utils/configure-store.js",
"src/utils/sort.js",
"package.json"
Expand Down
5 changes: 0 additions & 5 deletions yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -8943,11 +8943,6 @@ requires-port@^1.0.0:
resolved "https://registry.yarnpkg.com/requires-port/-/requires-port-1.0.0.tgz#925d2601d39ac485e091cf0da5c6e694dc3dcaff"
integrity sha1-kl0mAdOaxIXgkc8NpcbmlNw9yv8=

reselect@^4.1.6:
version "4.1.8"
resolved "https://registry.yarnpkg.com/reselect/-/reselect-4.1.8.tgz#3f5dc671ea168dccdeb3e141236f69f02eaec524"
integrity sha512-ab9EmR80F/zQTMNeneUr4cv+jSwPJgIlvEmVwLerwrWVbpLlBuls9XHzIeTFy4cegU2NHBp3va0LKOzU5qFEYQ==

resize-observer-polyfill@^1.5.1:
version "1.5.1"
resolved "https://registry.yarnpkg.com/resize-observer-polyfill/-/resize-observer-polyfill-1.5.1.tgz#0e9020dd3d21024458d4ebd27e23e40269810464"
Expand Down

0 comments on commit 4bdb8ac

Please sign in to comment.