Add typescript to elasticsearch service #2473

rolljee · 2023-09-06T07:57:05Z

What does this PR do ?

Add Typescript support to Elasticsearch.js file

kuzzle · 2023-09-06T07:57:50Z

Executed Checks

✔️ Branch Merge Check

✔️ Changelog Tag Check

✔️ PR Body Check

Generated by 🚫 dangerJS against 733d3f9

sebtiz13 · 2023-09-06T12:41:36Z

lib/types/storage/Elasticsearch.ts

+};
+
+export interface JSONObject {
+  [key: string]: JSONObject | any;


This union are always resolved to any because, any include JSONObject.
Also why not use JSONObject type from kuzzle-sdk package ?

I found it weird that kuzzle use types from it's sdk, it should be the other way around

Yes, maybe we should merge SDK JS in this repo to facilitate evolutions or generate a types package from the core and use it in SDK to avoid duplicated types in both packages

sebtiz13 · 2023-09-06T12:53:18Z

lib/service/storage/elasticsearch.ts

+    }: {
+      refresh?: string;
+      timeout?: number;
+      userId?: string;
+      injectKuzzleMeta?: boolean;
+      limits?: boolean;
+      source?: boolean;


Maybe define an interface with the requests options in lib/types/storage/ElasticSearch.ts and import it to use in methods parameters, it's more clear and lighter to read.

you are right

sebtiz13 · 2023-09-07T17:30:45Z

package.json

@@ -37,7 +37,7 @@
    "test:lint:ts": "eslint --max-warnings=0 ./lib --ext .ts --config .eslintc-ts.json",
    "test:lint": "npm run test:lint:js && npm run test:lint:ts",
    "test:unit:coverage": "DEBUG= nyc --reporter=text-summary --reporter=lcov mocha --exit",
-    "test:unit": "DEBUG= npx --node-arg=--trace-warnings mocha --exit",
+    "test:unit": "DEBUG= npx mocha --exit",


Normally npx are useless in npm scripts

…earch-typescript

sonarcloud · 2023-10-24T05:31:49Z

Kudos, SonarCloud Quality Gate passed!

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

No Coverage information
0.0% Duplication

alexandrebouthinon

It seems your local Prettier/ESLint vscode configuration broke the code formatting on the functional tests

alexandrebouthinon · 2023-11-20T09:19:17Z

test/mocks/elasticsearch.mock.js


-class ElasticsearchMock extends Elasticsearch {
+class ElasticsearchMock extends ElasticSearch {


I don't know if it's a good idea to update the Elasticsearch class name spelling

sonarcloud · 2023-12-06T15:10:45Z

Kudos, SonarCloud Quality Gate passed!

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

No Coverage information
0.0% Duplication

sebtiz13

Globaly can improve types, also why alternatively use JSONObject and Record<string, any> it's practically identical in result.

sebtiz13 · 2024-01-08T09:41:25Z

features-legacy/support/api/mqtt.js

Nit: Maybe your editor have bad configuration because final new line is generally required on projects

sebtiz13 · 2024-01-08T09:44:02Z

lib/types/storage/Elasticsearch.ts

+export interface JSONObject {
+  [key: string]: any;
+}


Why recreate JSONObject type here, instead of use the interface provided by kuzzle-sdk ?

sebtiz13 · 2024-01-08T09:52:43Z

lib/service/storage/elasticsearch.ts

    }

-    let documents = hits.hits.map((h) => ({ _id: h._id, _source: h._source }));
+    let documents = hits.hits.map((h: JSONObject) => ({


Suggested change

let documents = hits.hits.map((h: JSONObject) => ({

let documents = hits.hits.map((h: KDocument<KDocumentContentGeneric>) => ({

Why not use KDocument type instead of JSONObject ?

sebtiz13 · 2024-01-08T10:06:57Z

lib/service/storage/elasticsearch.ts

+  public _client: StorageClient;
+  public _scope: scopeEnum;
+  public _indexPrefix: string;
+  public _esWrapper: ESWrapper;
+  public _esVersion: any;
+  public _translator: QueryTranslator;


Generally the properties prefixed by an underscore are in private or protected

sebtiz13 · 2024-01-08T13:10:34Z

lib/types/storage/Elasticsearch.ts

+export type KRequestParams = {
+  refresh?: boolean | "wait_for";
+  timeout?: string;
+  userId?: string;
+  injectKuzzleMeta?: boolean;
+  limits?: boolean;
+  source?: boolean;
+};


Suggested change

export type KRequestParams = {

refresh?: boolean | "wait_for";

timeout?: string;

userId?: string;

injectKuzzleMeta?: boolean;

limits?: boolean;

source?: boolean;

};

interface KRequestParams {

userId?: string;

refresh?: boolean | "wait_for";

injectKuzzleMeta?: boolean;

retryOnConflict?: number;

timeout?: string;

}

It's better to use this interface to easily compose the params types in methods

sebtiz13 · 2024-01-08T13:28:58Z

lib/service/storage/elasticsearch.ts

    {
      refresh,
      timeout,
      userId = null,
      injectKuzzleMeta = true,
      limits = true,
      source = true,
-    } = {},
+    }: KRequestParams = {},


Suggested change

}: KRequestParams = {},

}: Exclude<KRequestParams, "retryOnConflict"> & { limits?: boolean; source?: boolean; } = {},

sebtiz13 · 2024-01-08T13:29:28Z

lib/service/storage/elasticsearch.ts

+    {
+      refresh = undefined,
+      retryOnConflict = 0,
+      timeout = undefined,
+      userId = null,
+    } = {},


Suggested change

{

refresh = undefined,

retryOnConflict = 0,

timeout = undefined,

userId = null,

} = {},

{

refresh = undefined,

retryOnConflict = 0,

timeout = undefined,

userId = null,

}: Exclude<KRequestParams, "injectKuzzleMeta"> = {},

sebtiz13 · 2024-01-08T13:30:09Z

lib/service/storage/elasticsearch.ts

+    {
+      refresh,
+      retryOnConflict = 0,
+      timeout,
+      userId = null,
+    }: {
+      refresh?: boolean | "wait_for";
+      retryOnConflict?: number;
+      timeout?: string;
+      userId?: string;
+    } = {},


Suggested change

{

refresh,

retryOnConflict = 0,

timeout,

userId = null,

}: {

refresh?: boolean | "wait_for";

retryOnConflict?: number;

timeout?: string;

userId?: string;

} = {},

{

refresh,

retryOnConflict = 0,

timeout,

userId = null,

}: Exclude<KRequestParams, "injectKuzzleMeta"> = {},

With new KRequestParams type we can compose type like it, to DRY

sebtiz13 · 2024-01-08T13:31:11Z

lib/service/storage/elasticsearch.ts

+    {
+      refresh,
+      timeout,
+      userId = null,
+    }: {
+      refresh?: boolean | "wait_for";
+      timeout?: string;
+      userId?: string;
+    } = {},


Suggested change

{

refresh,

timeout,

userId = null,

}: {

refresh?: boolean | "wait_for";

timeout?: string;

userId?: string;

} = {},

{

refresh,

timeout,

userId = null,

}: Exclude<KRequestParams, "injectKuzzleMeta" | "retryOnConflict"> = {},

With new KRequestParams type we can compose type like it, to DRY

sebtiz13 · 2024-01-08T13:32:03Z

lib/service/storage/elasticsearch.ts

+    {
+      refresh,
+    }: {
+      refresh?: boolean | "wait_for";
+      timeout?: number;
+    } = {},


Suggested change

{

refresh,

}: {

refresh?: boolean | "wait_for";

timeout?: number;

} = {},

{

refresh,

}: Pick<KRequestParams, "refresh" | "timeout"> = {},

With new KRequestParams type we can compose type like it, to DRY

kuzzle · 2024-01-16T14:51:33Z

🎉 This PR is included in version 2.29.0-beta.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

kuzzle · 2024-02-01T17:47:13Z

🎉 This PR is included in version 2.29.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

rolljee added 3 commits September 6, 2023 09:12

Add typescript to elasticsearch service

2c13783

Update default parameters

2396aad

Update docker compose

1d07b8d

rolljee added changelog:enhancements 2.x labels Sep 6, 2023

rolljee self-assigned this Sep 6, 2023

rolljee added 2 commits September 6, 2023 13:11

Update imports, lint & types

f333d5a

fix(lint): fixed lint and ts issues in elasticsearch file

4acba1f

sebtiz13 reviewed Sep 6, 2023

View reviewed changes

Typescript update

69f96cb

rolljee marked this pull request as draft September 7, 2023 08:45

rolljee added 5 commits September 7, 2023 13:35

Fixes tests on elasticsearch

6c95c7e

insert space (linting)

1de1bef

Update import

b4c3cb1

Testing

d138c33

Update to avoid breaking change

733d3f9

sebtiz13 reviewed Sep 7, 2023

View reviewed changes

rolljee added 11 commits September 13, 2023 09:54

Merge branch '2-dev' into feat/elasticsearch-typescript

f6f3e62

Update some commands

4414cfb

Merge ok

319cd62

Merge branch '2-dev' into feat/elasticsearch-typescript

613e69b

Merge 2-dev

fadf567

Functional tests legacy

7953f58

Update scrollId in features-legacy

c646f67

Taking time to make sonarcloud happy

20eaa6d

Mistake where made

0d41305

Sonarlint

677c686

Update types for linting and sonar

6128d70

rolljee marked this pull request as ready for review September 25, 2023 09:12

fmauNeko requested a review from sebtiz13 October 23, 2023 13:09

Merge branch '2-dev' of github.com:kuzzleio/kuzzle into feat/elastics…

1c1aa70

…earch-typescript

alexandrebouthinon requested changes Nov 20, 2023

View reviewed changes

rolljee linked an issue Nov 23, 2023 that may be closed by this pull request

Migrate Elasticsearch.js to Typescript to prepare futur work on ES8 #2501

Closed

rolljee added 8 commits November 29, 2023 11:50

Update classname to avoid modifying thing that don't need to

bf2e080

Revert some changes

f8b1463

Revert some changes

3f0ab19

Revert

739994b

Revert linting of some files

c6cc290

Revert linting

4cafa70

Revert

0e6b1cc

Revert Linting

4157839

rolljee added changelog:enhancements and removed changelog:enhancements 2.x labels Nov 29, 2023

Merge branch '2-dev' into feat/elasticsearch-typescript

9869be5

rolljee requested a review from alexandrebouthinon November 29, 2023 14:46

rolljee added 2 commits December 6, 2023 15:50

Merge branch '2-dev' into feat/elasticsearch-typescript

a98ac81

Lint the application

6b5addb

Kuruyia approved these changes Jan 3, 2024

View reviewed changes

alexandrebouthinon approved these changes Jan 8, 2024

View reviewed changes

rolljee merged commit 6907e43 into 2-dev Jan 8, 2024
25 checks passed

rolljee deleted the feat/elasticsearch-typescript branch January 8, 2024 13:33

sebtiz13 requested changes Jan 8, 2024

View reviewed changes

kuzzle added the released on @beta label Jan 16, 2024

kuzzle added the released This issue/pull request has been released. label Feb 1, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add typescript to elasticsearch service #2473

Add typescript to elasticsearch service #2473

rolljee commented Sep 6, 2023

kuzzle commented Sep 6, 2023 •

edited

sebtiz13 Sep 6, 2023

rolljee Sep 6, 2023

sebtiz13 Sep 7, 2023

sebtiz13 Sep 6, 2023

rolljee Sep 6, 2023

sebtiz13 Sep 7, 2023

sonarcloud bot commented Oct 24, 2023

alexandrebouthinon left a comment

alexandrebouthinon Nov 20, 2023

sonarcloud bot commented Dec 6, 2023

sebtiz13 left a comment

sebtiz13 Jan 8, 2024

sebtiz13 Jan 8, 2024

sebtiz13 Jan 8, 2024

sebtiz13 Jan 8, 2024

sebtiz13 Jan 8, 2024

sebtiz13 Jan 8, 2024

sebtiz13 Jan 8, 2024

sebtiz13 Jan 8, 2024

sebtiz13 Jan 8, 2024

sebtiz13 Jan 8, 2024

kuzzle commented Jan 16, 2024

kuzzle commented Feb 1, 2024


		class ElasticsearchMock extends Elasticsearch {
		class ElasticsearchMock extends ElasticSearch {

	let documents = hits.hits.map((h: JSONObject) => ({
	let documents = hits.hits.map((h: KDocument<KDocumentContentGeneric>) => ({

	}: KRequestParams = {},
	}: Exclude<KRequestParams, "retryOnConflict"> & { limits?: boolean; source?: boolean; } = {},

Add typescript to elasticsearch service #2473

Add typescript to elasticsearch service #2473

Conversation

rolljee commented Sep 6, 2023

What does this PR do ?

kuzzle commented Sep 6, 2023 • edited

Executed Checks

✔️ Branch Merge Check

✔️ Changelog Tag Check

✔️ PR Body Check

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

sonarcloud bot commented Oct 24, 2023

alexandrebouthinon left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sonarcloud bot commented Dec 6, 2023

sebtiz13 left a 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

Choose a reason for hiding this comment

kuzzle commented Jan 16, 2024

kuzzle commented Feb 1, 2024

kuzzle commented Sep 6, 2023 •

edited