Skip to content

Commit 27e01ed

Browse files
authored
Internal documentation (#442)
2 parents 5951ecd + 2b42fc9 commit 27e01ed

7 files changed

Lines changed: 325 additions & 13 deletions

File tree

MAINTENANCE.md

Lines changed: 7 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -92,19 +92,13 @@ Remember to run `npm i` in the `examples/` and `benchmark/` directories once you
9292
2. Bump the package version. Remember to update the version in `package-lock.json` in
9393
the main directory, `examples/`, and `benchmark/`. You can do this by running `npm i` in all 3 directories
9494
(see [example commit](https://github.com/scylladb/nodejs-rs-driver/pull/363/changes/41250609737052975129c7514439869324478008) on how to do that).
95-
3. Create a new tag.
96-
4. Ensure the extended CI passes.
97-
5. Create release notes on GitHub. The version tag must match version from `package.json` with `v` prefix (for example: `v0.2.0`).
95+
3. Add the new tag to the documentation configuration (see #470)
96+
4. Create a new tag.
97+
5. Ensure the extended CI passes.
98+
6. Create release notes on GitHub. The version tag must match version from `package.json` with `v` prefix (for example: `v0.2.0`).
9899
Once you publish release notes, CI action will trigger automatically. This action will build and publish the npm package.
99-
6. Once the CI action finishes, check if it succeeded. If it failed, you will have to fix the underlying issue, and re-run the CI action.
100-
7. Verify that the new release is visible at [npmjs site](https://www.npmjs.com/package/@scylladb/driver).
101-
8. Test the package, by installing it directly from npm. Go to `examples` directory, in `package.json` update the line:
102-
`"@scylladb/driver": "file:./../"`
103-
to:
104-
`"@scylladb/driver": "<just-released-version>"`,
105-
then run the following command:
106-
`npm i && node ./runner.js`
107-
<!-- The last step can potentially be set up as a CI action step. -->
100+
7. Once the CI action finishes, check if it succeeded. If it failed, you will have to fix the underlying issue, and re-run the CI action.
101+
8. Verify that the new release is visible at [npmjs site](https://www.npmjs.com/package/@scylladb/driver).
108102

109103
### Updating packages
110104

@@ -141,6 +135,7 @@ When converting existing code from JS to TS you can do it in 2 parts:
141135

142136
- Ensuring type safety within JS code,
143137
- Converting JS to TS.
138+
- Ensure empty `.npmignore` is present in the directory you just introduced the typescript file in.
144139

145140
The repository is set up in a way that supports files at all 3 conversion steps (including fully unconverted files).
146141

README.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
# ScyllaDB Node.js-RS Driver
1+
# ScyllaDB Node.js RS Driver
22

33
![test workflow](https://github.com/scylladb-zpp-2024-javascript-driver/scylladb-javascript-driver/actions/workflows/integration-tests.yml/badge.svg?branch=main)
44
![quality workflow](https://github.com/scylladb-zpp-2024-javascript-driver/scylladb-javascript-driver/actions/workflows/check-docs.yml/badge.svg?branch=main)

docs/source/internal/SUMMARY.md

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,35 @@
11
# Summary
22

3+
## Used shortcuts
4+
5+
Within this document (and internal code documentation) I may use informal names for different products.
6+
Here you can find a list of those names in all their variants, with the versions we decided on in user-facing parts of the driver.
7+
8+
JS - JavaScript
9+
TS - TypeScript
10+
11+
Node, NodeJs. Official: [Node.js](https://nodejs.org/)
12+
13+
Napi, NodeAPI, Napi-rs: see [napi.md](./napi.md#distinction). Official [Node-API](https://nodejs.org/api/n-api.html) and [NAPI-RS](https://napi.rs/).
14+
15+
DSx: Official: [DataStax](https://www.ibm.com/products/datastax)
16+
17+
DSx driver, old driver, DataStax driver, cassandra driver. Official:
18+
[cassandra-driver/cassandra-nodejs-driver](https://github.com/apache/cassandra-nodejs-driver)
19+
(The version depends on whether the language of the driver is obvious).
20+
This driver was initially developed by DSx and was transferred to Apache during development of this driver.
21+
For this reason some places may use the name referring to DSx. Any remaining public documentation should refer to this driver as
22+
`cassandra-driver`, with possible annotation like: `formerly known as DataStax Node.js Driver` (see main [README.md](../../../README.md)).
23+
24+
this driver, new driver, scylladb-driver, scylladb-driver-alpha. Official: ScyllaDB Node.js RS Driver / scylladb-driver-alpha / nodejs-rs-driver
25+
(standalone name / package name / repository name). Confusing? No worries, this repo probably breaks this convention quite a lot.
26+
27+
## Sections
28+
329
- [Errors](./error-throwing.md)
430
- [ParameterWrapper](./parameter-wrapper.md)
531
- [Query options overview](./query-options.md)
632
- [Logging](./logging.md)
33+
- [Napi](./napi.md)
34+
- [Miscellaneous](./miscellaneous.md)
35+
- [On types](./on-types.md)
Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,72 @@
1+
# Miscellaneous stuff
2+
3+
## DataStax API
4+
5+
DSx driver had some external APIs that were related to DSx-specific features:
6+
7+
- Custom auth providers
8+
- The entire datastax & geometry modules
9+
- Some client / query options
10+
11+
How did we handle those?
12+
13+
For functions/methods, we implemented stubs in JS code:
14+
15+
```js
16+
/**
17+
* @deprecated Not supported by the driver. Usage will throw an error.
18+
*/
19+
class Vertex {
20+
constructor() {
21+
throwNotSupported("Vertex");
22+
}
23+
}
24+
```
25+
26+
See [`structure.js`](../../../lib/datastax/graph/structure.js). The goal of those stubs is to loudly inform
27+
JS users of the driver that our driver does not support those features. This approach, while good for users transitioning
28+
from the old DSx driver, may provide some unnecessary noise for new users. When stabilizing the API in 1.0, those
29+
stubs can be removed, with proper documentation for transitioning users. (If there is such a need,
30+
you could create a temporary transition version for users transferring from the DSx driver, with those stubs enabled).
31+
32+
For the TS users of the driver, it was enough to remove those endpoints from the TS files.
33+
When someone attempts to use those features, they will get a compilation error,
34+
meaning we achieved the goal of explicitly informing the user of the deprecation of those endpoints.
35+
36+
When it comes to no longer supported options, we have a very similar approach:
37+
When a JS user provides a no-longer-supported option,
38+
[we throw an error](https://github.com/adespawn/nodejs/blob/ee439f78ed4b4e6c0a73b8aa2f649e45493ae1c5/lib/client-options.js#L971-L978).
39+
Those options are not mentioned in documentation, avoiding the noise for new users.
40+
When it comes to TS users, deleting those options from the TS API is again enough.
41+
42+
## Personal vendettas
43+
44+
Well, maybe not exactly personal, but still. This list is composed of small and big annoyances with the current state of the driver.
45+
46+
### API overloads
47+
48+
```js
49+
* @example <caption>Overloads</caption>
50+
* client.eachRow(query, rowCallback);
51+
* client.eachRow(query, params, rowCallback);
52+
* client.eachRow(query, params, options, rowCallback);
53+
* client.eachRow(query, params, rowCallback, callback);
54+
* client.eachRow(query, params, options, rowCallback, callback);
55+
```
56+
57+
From this [piece of code](https://github.com/adespawn/nodejs/blob/ee439f78ed4b4e6c0a73b8aa2f649e45493ae1c5/lib/client.js#L418-L423).
58+
59+
Allowing for overloading API in this way is (while perfectly fine by JS standards) very annoying when it comes to ensuring type safety.
60+
We have to guess based on the value types what overload the user is providing us with. But touching it will break the API.
61+
Is someone using those overloads? Idk. Will it be hard for users to remove usages of those overloads? Probably not.
62+
Feel free to re-visit this when creating 1.0.
63+
64+
### Long
65+
66+
The driver was initially developed when BigInt [was not a thing](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/BigInt#browser_compatibility).
67+
This means there are internal (and external) parts of the driver that use [Long](https://www.npmjs.com/package/long).
68+
This is the only JS dependency (excluding development dependencies) that the driver uses.
69+
While internal pruning of Long should be (probably annoying but still) possible, removing it from external parts of the driver
70+
may break some APIs. There is an issue related to this: [#41](https://github.com/scylladb/nodejs-rs-driver/issues/41).
71+
This topic would need to be re-visited before 1.0. You would have to decide how to approach this: keep Long as a legacy part of the API,
72+
or fully remove it?

docs/source/internal/napi.md

Lines changed: 161 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,161 @@
1+
# Napi (and napi-rs)
2+
3+
## Distinction
4+
5+
Node-API and napi-rs are two different things. When encountering just "napi" **in the documentation** (or PRs/issues),
6+
what is meant in most cases is Node-API (although there may be some exceptions, especially in old docs).
7+
However, when you encounter napi **in the code** (mostly Rust code, the napi crate), this refers to napi-rs.
8+
9+
In napi-rs there are different kinds of interfaces. Some of the interfaces napi-rs exposes are just 1-to-1 mappings from Node-API (mostly `napi::sys`).
10+
Then there are low-cost abstractions over Node-API, like `(To/From)NapiValue` traits and `napi::Env`.
11+
Then there are high-cost interfaces. Some of those add overhead that we would anyway need to introduce (like **sync** functions: functions with the `#[napi]` macro).
12+
Others add significant overhead that we can avoid by using lower-abstraction-level approaches to achieve sometimes very significant performance improvements.
13+
This often comes at the cost of some limitations, however those limitations rarely pose a problem for our use cases.
14+
15+
## Wrappers
16+
17+
There may be cases where you have some Rust value you want to work on, but the JS code does not need to have access to the members of such value,
18+
only its methods (think: [client session](https://github.com/adespawn/nodejs/blob/452f2acd2d8794161a1866c4b336df75c038cd22/src/session.rs#L29-L32)).
19+
In those cases you can create a wrapper that keeps the value private to Rust code and exposes methods through napi.
20+
21+
## Our interfaces
22+
23+
The main benefit of high-cost napi-rs interfaces is that they provide abstractions that very significantly reduce required verbosity.
24+
This section briefly describes the available internal interfaces and their use cases:
25+
26+
The napi-rs offers [napi-rs classes](https://napi.rs/docs/concepts/class). They offer a quite easy to understand interface.
27+
Using them is however [very](https://www.scylladb.com/wp-content/uploads/image10-5-768x768.png) [inefficient](https://www.scylladb.com/wp-content/uploads/image2-23-768x768.png),
28+
as we [have](https://github.com/scylladb/nodejs-rs-driver/pull/182) [discovered](https://github.com/scylladb/nodejs-rs-driver/pull/181).
29+
30+
So why is it so slow?
31+
32+
Creating an [Object](https://nodejs.org/api/n-api.html#napi_create_object) through Node-API is quite slow, and possibly even slower than doing the same from JS code
33+
(note: this is my recollection of things that happened over a year ago as of writing this documentation). Additionally, when [updating](https://github.com/scylladb/nodejs-rs-driver/pull/291)
34+
to [napi-v3](https://napi.rs/docs/more/v2-v3-migration-guide), our existing use cases stopped working due to
35+
[some problems](https://github.com/scylladb/nodejs-rs-driver/pull/291/commits/7c41208dc106f7ff20e1d0ffb9d33ecedd69e1ce) with BigInt.
36+
37+
To solve this problem, we have come up with a few workarounds, depending on the specific situation. For objects containing information, we design them
38+
not to have any methods on them. This is necessary since our solutions introduce some limitations at the benefit of better usability / performance in our specific use cases.
39+
40+
### To Napi Value
41+
42+
This section describes solutions for converting values from Rust to JS code.
43+
44+
#### Solutions for performance
45+
46+
As mentioned above, creation of JS [Objects](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Object) is quite expensive.
47+
What we have come up with, when returning multiple values from performance-critical paths, is to return a tuple ([array](https://nodejs.org/api/n-api.html#napi_create_array))
48+
of values we want to return.
49+
50+
You can find examples of such use cases in the [Rust part](https://github.com/scylladb/nodejs-rs-driver/blob/2e04a1701e4cfa039bac35c96d8b84b3a5f93867/src/paging.rs#L53-L73)
51+
of the code, as well as the [js parsing](https://github.com/scylladb/nodejs-rs-driver/blob/f4d6a68641bec162f066641b599eb31ec6efc0fa/lib/client.js#L354-L377)
52+
of such values.
53+
54+
#### Clean use for non critical paths
55+
56+
Sometimes we want to return objects with [clear structure](https://github.com/adespawn/nodejs/blob/452f2acd2d8794161a1866c4b336df75c038cd22/src/metadata/host.rs#L5-L12),
57+
where performance is not so critical (think: metadata). In this case helpers defined in [to_napi_obj.rs](../../../src/utils/to_napi_obj.rs) come to play.
58+
They allow you to convert:
59+
60+
- Rust structs and enums with values in variants to JS
61+
- Rust maps (String -> Any) into JS objects
62+
63+
Details on how to use them are present in the code docs for that file.
64+
Those helpers are a specialized replacement for [napi-rs classes](https://napi.rs/docs/concepts/class).
65+
They provide slightly better performance than napi-rs classes (since they do a bit less stuff on the returned object) and a reasonable user interface.
66+
67+
### From Napi Value
68+
69+
Similar to the previous section, we have helpers for converting JS objects to Rust structs. To see more, go to [from_napi_obj.rs](../../../src/utils/from_napi_obj.rs).
70+
This macro provides a much cleaner interface than [napi-rs classes](https://napi.rs/docs/concepts/class).
71+
Compared to napi-rs classes, it allows you to convert arbitrary JS objects with the desired structure, rather than only objects created through the napi-rs layer.
72+
73+
When using structs from this macro, all fields are converted from JS to Rust. This means that when you have a struct with a high number of fields but use only a few of them,
74+
you will still pay the cost of converting all unused fields.
75+
76+
### Casync (async bridge)
77+
78+
This is an open improvement that attempts to cut the very high CPU cost of synchronization in async functions.
79+
You can see the benchmarks for this feature in [#414](https://github.com/scylladb/nodejs-rs-driver/pull/414).
80+
TODO: finish this section once #414 is merged.
81+
82+
### JSResults
83+
84+
We use a custom JSResult for results that may end with an error. This replaces napi-rs Results, as those do not allow for custom errors.
85+
We need custom errors since the errors returned by the Rust driver contain additional information. For now we just pass the error name.
86+
We then need to use JSResult over regular Result since we cannot implement the ToNapiValue trait on a regular Result.
87+
This complicates the code slightly but is the solution we found with the lowest verbosity.
88+
89+
## Typing
90+
91+
When you pass values between JS and Rust it's nice to have it typed in some way, to improve both readability and correctness of the code.
92+
When using napi-classes or node-built in type representations (integers, bigInt, arrays, `Result`*, strings, buffers), in regular sync function,
93+
Napi-rs will generate type annotations correctly in the `index.d.ts` file upon compilation of the code.
94+
95+
\* result type is not "visible" across, since napi-rs does not annotate what the function can throw, but is properly handled as a passthrough type.
96+
97+
However, when using either custom (To/From)NapiValue implementation, one of the helper macros or casync functions, you will need to do a bit more work to properly annotate the types.
98+
Generally there are two things you need to do, in order to properly annotate those uses. What you need to do depends on the case.
99+
100+
- [`custom-types.d.ts`](../../../src/custom-types.d.ts): This file defines custom types. This means when you define new type, that you accept as argument or return from rust function,
101+
and it is **not** napi-class, you have define its JS(TS) type there.
102+
This file will then tell the JS/TS parser what does the magic type annotated in Rust function ex. `PagingResultWithExecutor` means.
103+
104+
So in the Rust code you just return `PagingResultWithExecutor`:
105+
106+
```rs
107+
// Simple version
108+
#[napi]
109+
pub async fn query_single_page_encoded(
110+
&self,
111+
...
112+
) -> Result<PagingResultWithExecutor> {
113+
114+
115+
// Full version with manual type override (see next point)
116+
#[napi(ts_return_type = "Promise<PagingResultWithExecutor>")]
117+
pub async fn query_single_page_encoded(
118+
&self,
119+
...
120+
) -> JsResult<PagingResultWithExecutor> {
121+
```
122+
123+
Napi-rs then annotates the TS definition of this function as returning `PagingResultWithExecutor`:
124+
125+
```ts
126+
export declare class SessionWrapper {
127+
...
128+
querySinglePage(...): Promise<PagingResultWithExecutor>
129+
}
130+
```
131+
132+
The custom definition in the `custom-types.d.ts`:
133+
134+
```ts
135+
export type PagingResultWithExecutor = [PagingStateWrapper | null, QueryResultWrapper, QueryExecutor]
136+
```
137+
138+
provides the meaning for this type. This is something you need to add manually to `custom-types.d.ts` file
139+
for every new type you add. This file will then be prepended to `index.d.ts` (used by JS/TS engine when parsing types in VScode) at the rust compilation time.
140+
141+
Note: this could be possibly automated with `pub trait TypeName` napi-rs trait.
142+
- Manual type overrides: When the napi-rs cannot properly deduce the JS name, you need to manually override the type in the endpoint that utilize it.
143+
By default, when you have a type `T` in Rust, and you were to have rust function return value of that type, it will be annotated in TS as `T`.
144+
Similarly, if you have a generic type: `G<T>` it will be annotated as `G<T>` in TS.
145+
146+
Here comes the problem: We have a type `JsResult<T>`, which should be opaque (the same way regular `Result<T>` is opaque) - meaning it should be annotated as
147+
`T` in TS (instead of the default `JsResult<T>`). Unfortunately you cannot handle it automatically.
148+
149+
To properly annotate such functions you need to use napi-rs `ts_return_type` for every function that returns such type.
150+
When using this tag, you need to specify the full type like this: `#[napi(ts_return_type = "Promise<PagingResult>")]`
151+
(meaning you need to remember about Promise part for async functions).
152+
153+
It's annoying, but allows to properly annotate the functions.
154+
You can see me question about this feature on the [napi-rs server](https://discord.com/channels/874290842444111882/874290843262021705/1505966134615085088).
155+
156+
## The napi problem
157+
158+
The napi-rs library is not perfect. We have already [found and fixed](https://github.com/napi-rs/napi-rs/issues?q=author%3Aadespawn) multiple bugs in the library.
159+
The library is developed with heavy LLM usage. The quality and readability of the code in some places is less than ideal.
160+
However, this is still the best option we have. To reduce the impact of problems with the napi-rs code, we avoid using high-abstraction parts of napi-rs,
161+
and where possible switch to self-written, specialized abstractions.

0 commit comments

Comments
 (0)