'import' syntax compatible and TypeScript declaration file #1038

radetsky · 2023-11-22T17:15:26Z

Added compatibility with import syntax to jsthemis
TypeScript declaration file
Increased the version of package.json
Updated CHANGELOG.md

Checklist

Change is covered by automated tests
Changelog is updated (in case of notable or breaking changes)

Lagovas · 2023-11-22T22:13:27Z

src/wrappers/themis/jsthemis/index.d.ts

@@ -0,0 +1,60 @@
+export declare class KeyPair {


did you write this file manually or somehow generated from the sources?

Manually. I did not find the tool that can read .node file to make a declaration file.

do we need constructor() here too similar to SymmetricKey? KeyPair looks like has constructor too -

themis/src/wrappers/themis/jsthemis/secure_keygen.cpp

Line 57 in f7c1f2d

void KeyPair::New(const Nan::FunctionCallbackInfo<v8::Value>& args)

And not obvious that it constructed from public + private

Lagovas · 2023-11-22T22:16:24Z

src/wrappers/themis/jsthemis/index.d.ts

+    constructor(peerID: Uint8Array, privateKey: Uint8Array, getPublicKeyCallback: GetPublicKeyCallback);
+    isEstablished(): boolean;
+    connectRequest(): Uint8Array;
+    sendToPeer(message: Uint8Array): void;


I didn't find these functions in secureSession.cpp to find result type. Where this function is declared?

Removed. Thank you. I did not write tests for SecureSession.

vixentael

@Lagovas looks good for you?

Lagovas

And I see, that you tried to add examples and update tests on CI but reverted changes. Did you met any problems with tests with new import syntax?

Lagovas · 2023-11-24T13:09:51Z

src/wrappers/themis/jsthemis/index.d.ts

@@ -0,0 +1,60 @@
+export declare class KeyPair {


do we need constructor() here too similar to SymmetricKey? KeyPair looks like has constructor too -

themis/src/wrappers/themis/jsthemis/secure_keygen.cpp

Line 57 in f7c1f2d

void KeyPair::New(const Nan::FunctionCallbackInfo<v8::Value>& args)

And not obvious that it constructed from public + private

Lagovas · 2023-11-24T13:13:52Z

src/wrappers/themis/jsthemis/index.d.ts

+    token: Uint8Array;
+}
+
+export declare class SecureCellTokenProtect {


Looks like TokenProtect has constructor that expects key too -

themis/src/wrappers/themis/jsthemis/secure_cell_token_protect.cpp

Line 64 in f7c1f2d

if (args.Length() < 1) {

or somehow used as WithKey.

I've checked. We do not announce that we can do it, but construction with new SecureCellTokenProtect(masterkey) will also work the same as SecureCellTokenProtect.withKey(masterKey). It wraps one function into a constructor.
I'm adding the constructor declaration.

vixentael · 2023-11-24T13:18:22Z

Should we add tests for this file?

radetsky · 2023-11-24T13:23:34Z

And I see, that you tried to add examples and update tests on CI but reverted changes. Did you met any problems with tests with new import syntax?

Nope, I met the issues with node >= 18. Decided to provide it later because of OpenSSL 3 issues.

Lagovas · 2023-11-24T16:59:28Z

Nope, I met the issues with node >= 18. Decided to provide it later because of OpenSSL 3 issues.

can we test it for openssl1.0/node <= 18?

added import_module.mjs and test

Typescript examples tests

Lagovas · 2023-11-28T00:23:39Z

docs/examples/ts/secure_cell.js

do we need this file here? looks like it's generated file from the .ts

Lagovas · 2023-11-28T00:30:05Z

.github/workflows/test-nodejs.yaml

+        run: |
+          cd $GITHUB_WORKSPACE/docs/examples/js/
+          echo "Test import syntax..."
+          node import_module.mjs


just to be sure, did you tried some failed script to check some failure case to be sure that it's not always echo ok?

I found documentation for run where described that by default shell used with -e flag. But real world can be unsynchronized from documentation and we met situations when tests ran but didn't exit with errors on failures.

Is it enough? Should I do a unit test?

Lagovas

lgtm. merge after removing redundant .js file as discussed

radetsky added 2 commits November 22, 2023 14:21

exports

7cafa69

Add TypeScript declaration file (index.d.ts)

abe6207

radetsky requested review from vixentael and Lagovas as code owners November 22, 2023 17:15

vixentael approved these changes Nov 22, 2023

View reviewed changes

Lagovas reviewed Nov 22, 2023

View reviewed changes

radetsky marked this pull request as draft November 23, 2023 10:21

radetsky changed the title ~~WIP: import syntax compatible and TypeScript declaration file~~ 'import' syntax compatible and TypeScript declaration file Nov 23, 2023

radetsky added 15 commits November 23, 2023 12:29

Remove non-existent methods

a1cdad8

Add example for TypeScript

473401d

Add flow for NodeJS with Typescript example

4864b00

restore test-nodejs.yaml

dfc73ca

add test-nodejs-openssl3.yaml

ede00cd

update test-nodejs-openssl3.yaml

c1d4275

update test-nodejs-openssl3.yaml

c0694b3

update test-nodejs-openssl3.yaml

688a7dc

update test-nodejs-openssl3.yaml

2bfd34a

update test-nodejs-openssl3.yaml

b19cc94

update test-nodejs-openssl3.yaml

671fc81

remove failed test-nodejs-openssl3.yaml

91a1275

move TS examples to another PR

a8c0865

restore previous test-nodejs.yaml

dcc1182

CHANGELOG.md updated

f7c1f2d

radetsky marked this pull request as ready for review November 24, 2023 09:12

vixentael approved these changes Nov 24, 2023

View reviewed changes

vixentael added the W-JsThemis 🍭 Wrapper: JsThemis for Node.js, JavaScript API, npm packages label Nov 24, 2023

Lagovas reviewed Nov 24, 2023

View reviewed changes

added two constructors

57cc538

radetsky added 12 commits November 27, 2023 14:36

added import_module.mjs and test

a7f1af8

remove not needed string

5f12c5b

Merge pull request #1 from radetsky/rad/import_module_mjs

576c69a

added import_module.mjs and test

ts tests 1

2c35b56

update workflow

5040c7b

update workflow

8fde76e

update workflow

dddc4ca

update workflow

1072e0f

downgrade typescript to 4.4 to be compatible with node 12

a9d0a5e

update workflow

8457223

update workflow

3afd6af

Merge pull request #2 from radetsky/rad/ts_tests

a393159

Typescript examples tests

radetsky requested a review from Lagovas November 27, 2023 18:21

Lagovas reviewed Nov 28, 2023

View reviewed changes

delete secure_cell.js from ts/

1f28271

Lagovas approved these changes Nov 28, 2023

View reviewed changes

radetsky merged commit 26665c0 into cossacklabs:release/0.15.0 Nov 28, 2023
10 checks passed

radetsky deleted the rad/jsthemis_exports branch December 7, 2023 13:06

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

'import' syntax compatible and TypeScript declaration file #1038

'import' syntax compatible and TypeScript declaration file #1038

radetsky commented Nov 22, 2023 •

edited

Loading

Lagovas Nov 22, 2023

radetsky Nov 23, 2023

Lagovas Nov 24, 2023

Lagovas Nov 22, 2023

radetsky Nov 23, 2023

vixentael left a comment

Lagovas left a comment

Lagovas Nov 24, 2023

Lagovas Nov 24, 2023

radetsky Nov 24, 2023

vixentael commented Nov 24, 2023

radetsky commented Nov 24, 2023

Lagovas commented Nov 24, 2023

Lagovas Nov 28, 2023

Lagovas Nov 28, 2023

radetsky Nov 28, 2023

Lagovas left a comment

'import' syntax compatible and TypeScript declaration file #1038

'import' syntax compatible and TypeScript declaration file #1038

Conversation

radetsky commented Nov 22, 2023 • edited Loading

Checklist

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

vixentael left a comment

Choose a reason for hiding this comment

Lagovas 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

vixentael commented Nov 24, 2023

radetsky commented Nov 24, 2023

Lagovas commented Nov 24, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Lagovas left a comment

Choose a reason for hiding this comment

radetsky commented Nov 22, 2023 •

edited

Loading