feat(js-client)!: Segregation of responsibility between js-client packages [fixes DXJ-525] #378

akim-bow · 2023-11-14T14:03:56Z

What have been done

Js-client doesn't care about aqua types anymore, interface simpler
added validation to builtins and public API
various bug fixes and optimizations

linear · 2023-11-14T14:03:59Z

DXJ-525 Ensure validation for js-client services

Need to add validation for:
1. builtin services

user services

packages/@tests/aqua/compile-aqua.ts

shamsartem · 2023-11-14T14:54:00Z

packages/core/aqua-to-js/src/generate/header.ts

+    FluencePeer as FluencePeer$$
+} from '@fluencelabs/js-client';
+
+${converters}`;


I don't think this is a good idea to hardcode the body of these functions in each compiled file
Don't understand the reason to do this
You already export v5_callFunction, v5_registerService and FluencePeer from '@fluencelabs/js-client' why not export these functions as well from '@fluencelabs/js-client' and use them normally

It went through hard refactoring process and end up in the aqua-to-js as i decided to completely remove any schema mentions from js-client. It uses schema to perform conversion. We have 3 options

Make an exception for this case (easier)

Minify and uglify this snippet to minimize it's impact on frontend (every function from every file would be bundled here)

Support batch compilation in aqua-to-js package. As a result every batch compilation will generate a single converter.js file

Decided to move it back to js-client

shamsartem · 2023-11-14T15:21:29Z

packages/@tests/aqua/src/index.ts


-    console.log("marine test finished, result: ", marine);
+    // TODO: Currently this behavior will timeout after TTL bcs this function don't call 'responseSrc'. Will be attended in the next updates


are you sure this matter can be safely moved to another issue? Is safe to release js-client without this fixed? If not - I would rather solve this issue before merging

Actually it wasn't working before. For aqua function without return values (no responseSrv calls) it's impossible to determine when function call should finish. After your comment I reviewed that, fixed this particular test with hacks. But it still will break on some cases in parallel air. To reliably fix this we need to wait for LNG-286

packages/core/aqua-to-js/package.json

shamsartem · 2023-11-14T15:44:22Z

packages/core/aqua-to-js/src/generate/converters.js

+    return null;
+  } else if (schema.tag === "option") {
+    if (!Array.isArray(value)) {
+      throw new Error("Bad schema");


I think a better error message could be useful, if you think a bit about when this error pottentially can happen. I would create an error text that explains what went wrong in general (e.g. Wasn't able to convert aqua to js) and then for each case some particulars that could be useful for debugging (e.g.schema generated by the aqua comiler expects an option and options are represented as js arrays but found type: ${typeof value}: ${value}) If aqua2ts recursive function also accepts the third argument which is context - then you can also store some context that can be useful to display in the error so it says how deep the error accured e.g. error happened at: someProperty.someNestedProperty.someNetedArray[0]

These errors are impossible actually. Only if you write your own wrong schema by hand and use it. But this is not the case. I think that no need to spend much efforts improving this. This made for satisfying ts, not for giving decent error messages.

These errors are tottally possible if something goes wrong with the compiler and for guys who write the compiler at least a little effort here would be useful to understand where the problem happened

Well, probably we can do some runtime validation here

shamsartem · 2023-11-14T15:52:10Z

packages/core/aqua-to-js/src/generate/interfaces.ts

-    ];
+    const registerServiceArgs =
+      // eslint-disable-next-line @typescript-eslint/consistent-type-assertions
+      (srvDef.defaultServiceId as DefaultServiceId).s_Some__f_value != null


this looks pretty hacky to be honest. Sometimes defaultServiceId is not a string and has some weird property in it? I would avoid this, should be possible somehow

I dunno how to avoid :)
It's actual value that comes from aqua compiler

We can discuss this someday with aqua team. I created a task

if this comes from the compiler why not put this in types instead of just putting string there and asserting the type?

Because these types are generated from compiler team :)

It's 1 more question to ask how to split these types. I'm feeling ok to keep them in aqia-api.

packages/core/aqua-to-js/src/generate/function.ts

shamsartem · 2023-11-14T16:05:54Z

packages/core/aqua-to-js/src/generate/function.ts

+    const argNames = [${argNames
+      .map((arg) => {
+        return `"${arg}"`;
+      })
+      .join(", ")}];
+    const argCount = argNames.length;
+    let peer = undefined;
+    if (args[0] instanceof FluencePeer$$) {
+        peer = args[0];
+        args = args.slice(1);
+    }
+
+
+    const callArgs = Object.fromEntries(args.slice(0, argCount).map((arg, i) => [argNames[i], arg]));
+
+    const params = ({
+        peer,
+        args: callArgs,
+        config: args[argCount]
+    });
+
+    const result = await callFunction$$({
+        script: ${scriptConstName},
+        ...params,
+    });


this part also feels strange to hard-code and duplicate for each compiled function
I believe there is no reason for compiled wrappers to contain any duplicated code
They should literally contatin only the functions to call with hardcoded air and schemas passed down to them
If you want to move this internally from one package to another for some reason - this is fine

if we are able to do ts inference from schemas some day then it would be probably enough to just compile aqua and save the result as JSON

{ "services": { "service1": { "schema": ... }, ... }, "functions": { "function1": { "schema": ... "air": ... } } }

and then on ts side

import someAqua from "compiled-aqua.json" import { aquaToJs } from "@fluencelabs/js-client" // service1 and function1 types are infered from compiled-aqua.json const { registerService1, function1 } = aquaToJs(someAqua)

so nothing has to be duplicated at all

or e.g additionally generate simple ts files that would just do something like this:

import { aquaToJs } from '@fluencelabs/js-client' const someCompiledAqua = { ... } export default aquaToJs(someCompiledAqua)

Removed all dublicated code

packages/core/aqua-to-js/package.json

shamsartem · 2023-11-17T15:42:30Z