Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Upgrade query-string to v8.1.0 | Add Wrapper function for queryString.parse() #1794

Merged
merged 10 commits into from
Sep 16, 2023
Merged

Upgrade query-string to v8.1.0 | Add Wrapper function for queryString.parse() #1794

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
4222f6d
add wrapper function for qs
anshgoyalevil Sep 15, 2023
5b9753f
fix tests and wrapper
anshgoyalevil Sep 15, 2023
8f018fd
Merge branch 'main' into qs-upgrade
anshgoyalevil Sep 15, 2023
ab59276
revert parseQuery.test.ts to parseQuery.ts
anshgoyalevil Sep 15, 2023
0bc3604
Merge branch 'qs-upgrade' of https://github.com/anshgoyalevil/jaeger-…
anshgoyalevil Sep 15, 2023
c7cebbd
add default mock
anshgoyalevil Sep 15, 2023
9078f7f
rename file to tsx
anshgoyalevil Sep 15, 2023
7736953
Merge branch 'main' into qs-upgrade
anshgoyalevil Sep 16, 2023
1c3d81e
add unit test
anshgoyalevil Sep 16, 2023
fe039b8
add license header
anshgoyalevil Sep 16, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion packages/jaeger-ui/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@
"moment": "^2.18.1",
"object-hash": "^3.0.0",
"prop-types": "^15.5.10",
"query-string": "^6.3.0",
"query-string": "^8.1.0",
"raven-js": "^3.22.1",
"react": "^18.2.0",
"react-circular-progressbar": "^2.1.0",
Expand Down
4 changes: 2 additions & 2 deletions packages/jaeger-ui/src/components/DeepDependencies/url.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,10 @@
// See the License for the specific language governing permissions and
// limitations under the License.

import queryString from 'query-string';
import * as reactRouterDom from 'react-router-dom';

import { ROUTE_PATH, matches, getUrl, getUrlState, sanitizeUrlState } from './url';
import * as parseQuery from '../../utils/parseQuery';

jest.mock('react-router-dom', () => ({
matchPath: jest.fn(),
Expand Down Expand Up @@ -111,7 +111,7 @@ describe('DeepDependencyGraph/url', () => {
let parseSpy;

beforeAll(() => {
parseSpy = jest.spyOn(queryString, 'parse');
parseSpy = jest.spyOn(parseQuery, 'default');
warnSpy = jest.spyOn(console, 'warn').mockImplementation();
});

Expand Down
5 changes: 3 additions & 2 deletions packages/jaeger-ui/src/components/DeepDependencies/url.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,9 @@

import _isEmpty from 'lodash/isEmpty';
import memoizeOne from 'memoize-one';
import queryString from 'query-string';
import { matchPath } from 'react-router-dom';
import queryString from 'query-string';
yurishkuro marked this conversation as resolved.
Show resolved Hide resolved
import parseQuery from '../../utils/parseQuery';

import { EDdgDensity, TDdgSparseUrlState } from '../../model/ddg/types';
import prefixUrl from '../../utils/prefix-url';
Expand Down Expand Up @@ -62,7 +63,7 @@ export const getUrlState = memoizeOne(function getUrlState(search: string): TDdg
showOp,
start,
visEncoding,
} = queryString.parse(search);
} = parseQuery(search);
const rv: TDdgSparseUrlState = {
density: firstParam(density) as EDdgDensity,
};
Expand Down
3 changes: 2 additions & 1 deletion packages/jaeger-ui/src/components/SearchTracePage/url.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import prefixUrl from '../../utils/prefix-url';
import { MAX_LENGTH } from '../DeepDependencies/Graph/DdgNodeContent/constants';

import { SearchQuery } from '../../types/search';
import parseQuery from '../../utils/parseQuery';

function eqEq(a: string | number | null | undefined, b: string | number | null | undefined) {
return (a == null && b == null) || String(a) === String(b);
Expand Down Expand Up @@ -69,7 +70,7 @@ export function getUrl(query?: TUrlState) {
export const getUrlState: (search: string) => TUrlState = memoizeOne(function getUrlState(
search: string
): TUrlState {
const { traceID, span, ...rest } = queryString.parse(search);
const { traceID, span, ...rest } = parseQuery(search);
const rv: TUrlState = { ...rest };
const traceIDs = new Set(!traceID || Array.isArray(traceID) ? traceID : [traceID]);
const spanLinks: Record<string, string> = {};
Expand Down
3 changes: 2 additions & 1 deletion packages/jaeger-ui/src/components/TraceDiff/TraceDiff.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ import TTraceDiffState from '../../types/TTraceDiffState';
import pluckTruthy from '../../utils/ts/pluckTruthy';

import './TraceDiff.css';
import parseQuery from '../../utils/parseQuery';

type TStateProps = {
a: string | undefined;
Expand Down Expand Up @@ -165,7 +166,7 @@ export class TraceDiffImpl extends React.PureComponent<TStateProps & TDispatchPr
// TODO(joe): simplify but do not invalidate the URL
export function mapStateToProps(state: ReduxState, ownProps: { match: match<TDiffRouteParams> }) {
const { a, b } = ownProps.match.params;
const { cohort: origCohort = [] } = queryString.parse(state.router.location.search);
const { cohort: origCohort = [] } = parseQuery(state.router.location.search);
const fullCohortSet: Set<string> = new Set(pluckTruthy([a, b].concat(origCohort)));
const cohort: string[] = Array.from(fullCohortSet);
const { traces } = state.trace;
Expand Down
4 changes: 2 additions & 2 deletions packages/jaeger-ui/src/components/common/UiFindInput.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,18 +17,18 @@ import { shallow } from 'enzyme';
import { Icon, Input } from 'antd';
import { CloseOutlined } from '@ant-design/icons';
import debounceMock from 'lodash/debounce';
import queryString from 'query-string';

import { UnconnectedUiFindInput, extractUiFindFromState } from './UiFindInput';
import updateUiFindSpy from '../../utils/update-ui-find';
import * as parseQuery from '../../utils/parseQuery';

jest.mock('lodash/debounce');

jest.mock('../../utils/update-ui-find');

describe('UiFind', () => {
const flushMock = jest.fn();
const queryStringParseSpy = jest.spyOn(queryString, 'parse');
const queryStringParseSpy = jest.spyOn(parseQuery, 'default');

const uiFind = 'uiFind';
const ownInputValue = 'ownInputValue';
Expand Down
4 changes: 2 additions & 2 deletions packages/jaeger-ui/src/components/common/UiFindInput.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,12 +18,12 @@ import { CloseOutlined } from '@ant-design/icons';
import { History as RouterHistory, Location } from 'history';
import _debounce from 'lodash/debounce';
import _isString from 'lodash/isString';
import queryString from 'query-string';
import { connect } from 'react-redux';
import { RouteComponentProps, withRouter } from 'react-router-dom';

import updateUiFind from '../../utils/update-ui-find';
import { TNil, ReduxState } from '../../types/index';
import parseQuery from '../../utils/parseQuery';

type TOwnProps = RouteComponentProps<any> & {
allowClear?: boolean;
Expand Down Expand Up @@ -110,7 +110,7 @@ export class UnconnectedUiFindInput extends React.PureComponent<TProps, StateTyp
}

export function extractUiFindFromState(state: ReduxState): TExtractUiFindFromStateReturn {
const { uiFind: uiFindFromUrl } = queryString.parse(state.router.location.search);
const { uiFind: uiFindFromUrl } = parseQuery(state.router.location.search);
const uiFind = Array.isArray(uiFindFromUrl) ? uiFindFromUrl.join(' ') : uiFindFromUrl;
return { uiFind };
}
Expand Down
37 changes: 37 additions & 0 deletions packages/jaeger-ui/src/utils/parseQuery.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
// Copyright (c) 2019 Uber Technologies, Inc.
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

import queryString, { ParseOptions } from 'query-string';

interface IParsedQuery {
[key: string]: string | null | (string | null)[];
}

function parseQuery(query: string, options?: ParseOptions): { [key: string]: string | string[] } {
const parsed: IParsedQuery = queryString.parse(query, options);

const result: { [key: string]: string | string[] } = {};

Object.keys(parsed).forEach(key => {
if (Array.isArray(parsed[key])) {
result[key] = (parsed[key] as string[]).map((item: string | null) => item || '');
} else {
result[key] = parsed[key] as string | string[];
}
});

return result;
}

export default parseQuery;
4 changes: 2 additions & 2 deletions packages/jaeger-ui/src/utils/tracking/ga.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@
// limitations under the License.

import _get from 'lodash/get';
import queryString from 'query-string';
import ReactGA from 'react-ga';
import Raven, { RavenOptions, RavenTransportOptions } from 'raven-js';

Expand All @@ -23,6 +22,7 @@ import { Config } from '../../types/config';
import { IWebAnalyticsFunc } from '../../types/tracking';
import { logTrackingCalls } from './utils';
import { getAppEnvironment, shouldDebugGoogleAnalytics } from '../constants';
import parseQuery from '../parseQuery';

const isTruish = (value?: string | string[]) => {
return Boolean(value) && value !== '0' && value !== 'false';
Expand All @@ -35,7 +35,7 @@ const GA: IWebAnalyticsFunc = (config: Config, versionShort: string, versionLong
const isTest = appEnv === 'test';
const isDebugMode =
(isDev && isTruish(shouldDebugGoogleAnalytics())) ||
isTruish(queryString.parse(_get(window, 'location.search'))['ga-debug']);
isTruish(parseQuery(_get(window, 'location.search'))['ga-debug']);
const gaID = _get(config, 'tracking.gaID');
const isErrorsEnabled = isDebugMode || Boolean(_get(config, 'tracking.trackErrors'));
const cookiesToDimensions = _get(config, 'tracking.cookiesToDimensions');
Expand Down
1 change: 1 addition & 0 deletions packages/jaeger-ui/tsconfig.lint.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@
"src/reducers/trace.js",
"src/selectors/dependencies.js",
"src/utils/configure-store.js",
"src/utils/parseQuery.ts",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this needed? Don't we pick up *ts files for linting automatically? If we don't it's a very sad state - linting should be on by default.

Copy link
Member Author

@anshgoyalevil anshgoyalevil Sep 15, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not actually. We aren't picking up *ts files automatically. We are doing it for *.tsx files.

Without this rule:
image

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

let's rename the file to tsx then, for now. I think we need to fix this separately.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done!

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Though, it can also be configured in tsconfig.json

{
    "extends": "../../tsconfig",
-   "include": ["src/**/*.tsx", "typings"]
+  "include": ["src/**/*.tsx", "src/**/*.ts", "typings"]
}

"src/utils/sort.js",
"package.json"
]
Expand Down
68 changes: 48 additions & 20 deletions yarn.lock
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2616,12 +2616,19 @@
resolved "https://registry.yarnpkg.com/@types/range-parser/-/range-parser-1.2.4.tgz#cd667bcfdd025213aafb7ca5915a932590acdcdc"
integrity sha512-EEhsLsD6UsDM1yFhAvy0Cjr6VwmpMWqFBCb9w07wVugF7w9nfajxLuVmngTIpgS6svCnm6Vaw+MZhoDCKnOfsw==

"@types/react-dom@16.9.18", "@types/react-dom@18.2.5", "@types/react-dom@^18.0.0":
version "16.9.18"
resolved "https://registry.yarnpkg.com/@types/react-dom/-/react-dom-16.9.18.tgz#1fda8b84370b1339d639a797a84c16d5a195b419"
integrity sha512-lmNARUX3+rNF/nmoAFqasG0jAA7q6MeGZK/fdeLwY3kAA4NPgHHrG5bNQe2B5xmD4B+x6Z6h0rEJQ7MEEgQxsw==
"@types/react-dom@18.2.5":
version "18.2.5"
resolved "https://registry.yarnpkg.com/@types/react-dom/-/react-dom-18.2.5.tgz#5c5f13548bda23cd98f50ca4a59107238bfe18f3"
integrity sha512-sRQsOS/sCLnpQhR4DSKGTtWFE3FZjpQa86KPVbhUqdYMRZ9FEFcfAytKhR/vUG2rH1oFbOOej6cuD7MFSobDRQ==
dependencies:
"@types/react" "^16"
"@types/react" "*"

"@types/react-dom@^18.0.0":
version "18.2.7"
resolved "https://registry.yarnpkg.com/@types/react-dom/-/react-dom-18.2.7.tgz#67222a08c0a6ae0a0da33c3532348277c70abb63"
integrity sha512-GRaAEriuT4zp9N4p1i8BDBYmEyfo+xQ3yHjJU4eiK5NDa1RmUZG+unZABUTK4/Ox/M+GaHwb6Ow8rUITrtjszA==
dependencies:
"@types/react" "*"

"@types/react-helmet@^6.1.5":
version "6.1.5"
Expand Down Expand Up @@ -2659,7 +2666,7 @@
dependencies:
"@types/react" "*"

"@types/react@*", "@types/[email protected]", "@types/[email protected]", "@types/react@^16":
"@types/react@*", "@types/[email protected]":
version "16.14.35"
resolved "https://registry.yarnpkg.com/@types/react/-/react-16.14.35.tgz#9d3cf047d85aca8006c4776693124a5be90ee429"
integrity sha512-NUEiwmSS1XXtmBcsm1NyRRPYjoZF2YTE89/5QiLt5mlGffYK9FQqOKuOLuXNrjPQV04oQgaZG+Yq02ZfHoFyyg==
Expand All @@ -2668,6 +2675,14 @@
"@types/scheduler" "*"
csstype "^3.0.2"

"@types/[email protected]":
version "16.8.7"
resolved "https://registry.yarnpkg.com/@types/react/-/react-16.8.7.tgz#7b1c0223dd5494f9b4501ad2a69aa6acb350a29b"
integrity sha512-0xbkIyrDNKUn4IJVf8JaCn+ucao/cq6ZB8O6kSzhrJub1cVSqgTArtG0qCfdERWKMEIvUbrwLXeQMqWEsyr9dA==
dependencies:
"@types/prop-types" "*"
csstype "^2.2.0"

"@types/[email protected]":
version "2.2.1"
resolved "https://registry.yarnpkg.com/@types/redux-actions/-/redux-actions-2.2.1.tgz#c1f4a7283ecd3cd696291550361e441bf9389370"
Expand Down Expand Up @@ -4527,6 +4542,11 @@ cssstyle@^3.0.0:
dependencies:
rrweb-cssom "^0.6.0"

csstype@^2.2.0:
version "2.6.21"
resolved "https://registry.yarnpkg.com/csstype/-/csstype-2.6.21.tgz#2efb85b7cc55c80017c66a5ad7cbd931fda3a90e"
integrity sha512-Z1PhmomIfypOpoMjRQB70jfvy/wxT50qW08YXO5lMIJkrdq4yOTR+AW7FqutScmB9NkLwxo+jU+kZLbofZZq/w==

csstype@^3.0.2:
version "3.1.2"
resolved "https://registry.yarnpkg.com/csstype/-/csstype-3.1.2.tgz#1d4bf9d572f11c14031f0436e1c10bc1f571f50b"
Expand Down Expand Up @@ -4806,10 +4826,10 @@ decimal.js@^10.4.2, decimal.js@^10.4.3:
resolved "https://registry.yarnpkg.com/decimal.js/-/decimal.js-10.4.3.tgz#1044092884d245d1b7f65725fa4ad4c6f781cc23"
integrity sha512-VBBaLc1MgL5XpzgIP7ny5Z6Nx3UrRkIViUkPUdtl9aya5amy3De1gsUUSB1g3+3sExYNjCAsAznmukyxCb1GRA==

decode-uri-component@^0.2.0:
version "0.2.2"
resolved "https://registry.yarnpkg.com/decode-uri-component/-/decode-uri-component-0.2.2.tgz#e69dbe25d37941171dd540e024c444cd5188e1e9"
integrity sha512-FqUYQ+8o158GyGTrMFJms9qh3CqTKvAqgqsTnkLI8sKu0028orqBhxNMFkFen0zGyg6epACD32pjVk58ngIErQ==
decode-uri-component@^0.4.1:
version "0.4.1"
resolved "https://registry.yarnpkg.com/decode-uri-component/-/decode-uri-component-0.4.1.tgz#2ac4859663c704be22bf7db760a1494a49ab2cc5"
integrity sha512-+8VxcR21HhTy8nOt6jf20w0c9CADrw1O8d+VZ/YzzCt4bJ3uBjw+D1q2osAB8RnpwwaeYBxy0HyKQxD5JBMuuQ==

[email protected]:
version "0.7.0"
Expand Down Expand Up @@ -6004,6 +6024,11 @@ fill-range@^7.0.1:
dependencies:
to-regex-range "^5.0.1"

filter-obj@^5.1.0:
version "5.1.0"
resolved "https://registry.yarnpkg.com/filter-obj/-/filter-obj-5.1.0.tgz#5bd89676000a713d7db2e197f660274428e524ed"
integrity sha512-qWeTREPoT7I0bifpPUXtxkZJ1XJzxWtfoWWkdVGqa+eCr3SHW/Ocp89o8vLvbUuQnadybJpjOKu4V+RwO6sGng==

[email protected]:
version "1.2.0"
resolved "https://registry.yarnpkg.com/finalhandler/-/finalhandler-1.2.0.tgz#7d23fe5731b207b4640e4fcd00aec1f9207a7b32"
Expand Down Expand Up @@ -8341,7 +8366,7 @@ lodash.upperfirst@^4.3.1:
resolved "https://registry.yarnpkg.com/lodash.upperfirst/-/lodash.upperfirst-4.3.1.tgz#1365edf431480481ef0d1c68957a5ed99d49f7ce"
integrity sha512-sReKOYJIJf74dhJONhU4e0/shzi1trVbSWDOhKYE5XV2O+H7Sb2Dihwuc7xWxVl+DgFPyTqIN3zMfT9cq5iWDg==

lodash@4.17.21, lodash@^4.16.5, lodash@^4.17.15, lodash@^4.17.19, lodash@^4.17.20, lodash@^4.17.21, lodash@^4.17.4, lodash@^4.2.1:
lodash@^4.16.5, lodash@^4.17.15, lodash@^4.17.19, lodash@^4.17.20, lodash@^4.17.21, lodash@^4.17.4, lodash@^4.2.1:
version "4.17.21"
resolved "https://registry.yarnpkg.com/lodash/-/lodash-4.17.21.tgz#679591c564c3bffaae8454cf0b3df370c3d6911c"
integrity sha512-v2kDEe57lecTulaDIuNTPy3Ry4gLGJ6Z1O3vE1krgXZNrsQ+LFTGHVxVjcXPs17LhbZVGedAJv8XZ1tvj5FvSg==
Expand Down Expand Up @@ -9941,12 +9966,14 @@ [email protected]:
dependencies:
side-channel "^1.0.4"

query-string@^6.3.0:
version "6.3.0"
resolved "https://registry.yarnpkg.com/query-string/-/query-string-6.3.0.tgz#41ae8a61e1213c80b182d5db6cf129e05af89fc5"
query-string@^8.1.0:
version "8.1.0"
resolved "https://registry.yarnpkg.com/query-string/-/query-string-8.1.0.tgz#e7f95367737219544cd360a11a4f4ca03836e115"
integrity sha512-BFQeWxJOZxZGix7y+SByG3F36dA0AbTy9o6pSmKFcFz7DAj0re9Frkty3saBn3nHo3D0oZJ/+rx3r8H8r8Jbpw==
dependencies:
decode-uri-component "^0.2.0"
strict-uri-encode "^2.0.0"
decode-uri-component "^0.4.1"
filter-obj "^5.1.0"
split-on-first "^3.0.0"

querystringify@^2.1.1:
version "2.2.0"
Expand Down Expand Up @@ -11497,6 +11524,11 @@ spdy@^4.0.2:
select-hose "^2.0.0"
spdy-transport "^3.0.0"

split-on-first@^3.0.0:
version "3.0.0"
resolved "https://registry.yarnpkg.com/split-on-first/-/split-on-first-3.0.0.tgz#f04959c9ea8101b9b0bbf35a61b9ebea784a23e7"
integrity sha512-qxQJTx2ryR0Dw0ITYyekNQWpz6f8dGd7vffGNflQQ3Iqj9NJ6qiZ7ELpZsJ/QBhIVAiDfXdag3+Gp8RvWa62AA==

split2@^3.2.2:
version "3.2.2"
resolved "https://registry.yarnpkg.com/split2/-/split2-3.2.2.tgz#bf2cf2a37d838312c249c89206fd7a17dd12365f"
Expand Down Expand Up @@ -11567,10 +11599,6 @@ store@^2.0.12:
version "2.0.12"
resolved "https://registry.yarnpkg.com/store/-/store-2.0.12.tgz#8c534e2a0b831f72b75fc5f1119857c44ef5d593"

strict-uri-encode@^2.0.0:
version "2.0.0"
resolved "https://registry.yarnpkg.com/strict-uri-encode/-/strict-uri-encode-2.0.0.tgz#b9c7330c7042862f6b142dc274bbcc5866ce3546"

string-convert@^0.2.0:
version "0.2.1"
resolved "https://registry.yarnpkg.com/string-convert/-/string-convert-0.2.1.tgz#6982cc3049fbb4cd85f8b24568b9d9bf39eeff97"
Expand Down