-
Notifications
You must be signed in to change notification settings - Fork 919
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.
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
[Console] Converted all /lib/autocomplete/**/*.js
files to typescript
#4148
Changes from 6 commits
71c8ece
f9e615d
c372e1c
78bed2b
bf6a7e4
75c7d94
07598c4
f6a8cee
9375923
40ed198
5156d6e
d9e3e0e
31b40ad
ad97705
2d172ff
1205e53
ec4a3ea
84fb3f9
801f4af
148202c
6a70af1
570e901
5c24c38
75ca742
b6aedd1
e9581d8
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -209,6 +209,7 @@ Inspired from [Keep a Changelog](https://keepachangelog.com/en/1.0.0/) | |
- [Console] Remove unused ul element and its custom styling ([#3993](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/3993)) | ||
- Fix EUI/OUI type errors ([#3798](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/3798)) | ||
- Remove unused Sass in `tile_map` plugin ([#4110](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/4110)) | ||
- [Console] Convert /lib/autocomplete/ to TypeScript ([#4148](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/4148)) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Maybe: [Console] Migrating There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah, I think it will be better description, I updated changelog |
||
|
||
### 🔩 Tests | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -31,27 +31,36 @@ | |
import _ from 'lodash'; | ||
import { UrlParams } from '../../autocomplete/url_params'; | ||
import { populateContext } from '../../autocomplete/engine'; | ||
import { AutoCompleteContext, Term } from '../types'; | ||
import { PartialAutoCompleteContext } from '../components/autocomplete_component'; | ||
import { SharedComponent } from '../components'; | ||
|
||
describe('Url params', () => { | ||
function paramTest(name, description, tokenPath, expectedContext, globalParams) { | ||
function paramTest( | ||
name: string, | ||
description: Record<string, unknown>, | ||
tokenPath: string | Array<string | string[]>, | ||
expectedContext: PartialAutoCompleteContext, | ||
globalParams?: Record<string, unknown> | ||
) { | ||
test(name, function () { | ||
const urlParams = new UrlParams(description, globalParams || {}); | ||
if (typeof tokenPath === 'string') { | ||
tokenPath = _.map(tokenPath.split('/'), function (p) { | ||
p = p.split(','); | ||
if (p.length === 1) { | ||
return p[0]; | ||
const pSplit = p.split(','); | ||
if (pSplit.length === 1) { | ||
return pSplit[0]; | ||
} | ||
return p; | ||
return pSplit; | ||
}); | ||
} | ||
|
||
if (expectedContext.autoCompleteSet) { | ||
expectedContext.autoCompleteSet = _.map(expectedContext.autoCompleteSet, function (t) { | ||
if (_.isString(t)) { | ||
t = { name: t }; | ||
expectedContext.autoCompleteSet = _.map(expectedContext.autoCompleteSet, function (term) { | ||
if (_.isString(term)) { | ||
term = { name: term }; | ||
} | ||
return t; | ||
return term; | ||
}); | ||
expectedContext.autoCompleteSet = _.sortBy(expectedContext.autoCompleteSet, 'name'); | ||
} | ||
|
@@ -63,32 +72,34 @@ describe('Url params', () => { | |
context, | ||
null, | ||
expectedContext.autoCompleteSet, | ||
urlParams.getTopLevelComponents() | ||
urlParams.getTopLevelComponents() as SharedComponent[] | ||
); | ||
|
||
if (context.autoCompleteSet) { | ||
context.autoCompleteSet = _.sortBy(context.autoCompleteSet, 'name'); | ||
const populatedContext = context as AutoCompleteContext; | ||
|
||
if (populatedContext.autoCompleteSet) { | ||
populatedContext.autoCompleteSet = _.sortBy(populatedContext.autoCompleteSet, 'name'); | ||
} | ||
|
||
expect(context).toEqual(expectedContext); | ||
expect(populatedContext).toEqual(expectedContext); | ||
}); | ||
} | ||
|
||
function t(name, meta, insertValue) { | ||
let r = name; | ||
function t(name: string, meta?: string, insertValue?: string) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. deserve a better name. |
||
let r: string | Term = name; | ||
if (meta) { | ||
r = { name: name, meta: meta }; | ||
r = { name, meta }; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. since mutates r here, may could do
or even simply more ... There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Agree, I have updated it and its name. |
||
if (meta === 'param' && !insertValue) { | ||
insertValue = name + '='; | ||
} | ||
} | ||
if (insertValue) { | ||
if (_.isString(r)) { | ||
r = { name: name }; | ||
r = { name }; | ||
} | ||
r.insertValue = insertValue; | ||
} | ||
return r; | ||
return r as Term; | ||
} | ||
|
||
(function () { | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems you include ur other PRs. CHANGELOG should only consider your current PR. For example, you should only include #4148. This might cause some future merge conflicts. For example if you merge #3798 first, then you would need to solve the conflict before merging this one. Including changes that are unrelated to the PR's can lead to confusion to others.