feat: JS client #3658

nasdf · 2025-05-02T18:25:53Z

Relevant issue(s)

Resolves #2442

Description

This PR adds a JS client implementation to DefraDB.

Sorry for the messy commit history. I accidentally started this on a previous feature branch.

Tasks

I made sure the code is well commented, particularly hard-to-understand areas.
I made sure the repository-held documentation is changed accordingly.
I made sure the pull request title adheres to the conventional commit style (the subset used in the project can be found in tools/configs/chglog/config.yml).
I made sure to discuss its limitations such as threats to validity, vulnerability to mistake and misuse, robustness to invalidation of assumptions, resource requirements, ...

How has this been tested?

Added integration test client wrapper.

Specify the platform(s) on which this was tested:

MacOS

nasdf · 2025-05-02T18:27:17Z

tests/integration/subscription/subscription_test.go

-			},
-		},
-	}
+// func TestSubscriptionWithUpdateAllMutations(t *testing.T) {


todo: uncomment when fixed

codecov · 2025-05-02T18:46:39Z

Codecov Report

Attention: Patch coverage is 42.07459% with 497 lines in your changes missing coverage. Please review.

Project coverage is 77.10%. Comparing base (1281b6a) to head (52f44ca).
Report is 1 commits behind head on develop.

Files with missing lines	Patch %	Lines
js/client_collection.go	38.82%	120 Missing and 25 partials ⚠️
js/client_store.go	44.91%	88 Missing and 31 partials ⚠️
js/client_db.go	33.33%	68 Missing and 20 partials ⚠️
js/client_lens.go	0.00%	77 Missing ⚠️
js/utils.go	50.60%	30 Missing and 11 partials ⚠️
js/js.go	0.00%	20 Missing ⚠️
client/document.go	50.00%	2 Missing and 1 partial ⚠️
js/client.go	91.67%	2 Missing and 1 partial ⚠️
cli/start.go	80.00%	1 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff             @@
##           develop    #3658      +/-   ##
===========================================
- Coverage    78.01%   77.10%   -0.91%     
===========================================
  Files          405      418      +13     
  Lines        37341    38197     +856     
===========================================
+ Hits         29129    29451     +322     
- Misses        6431     6872     +441     
- Partials      1781     1874      +93

Flag	Coverage Δ
all-tests	`77.10% <42.07%> (-0.91%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines	Coverage Δ
acp/identity/identity.go	`70.80% <ø> (ø)`
cli/utils.go	`67.57% <100.00%> (ø)`
client/db.go	`89.29% <ø> (ø)`
errors/errors.go	`100.00% <ø> (ø)`
errors/stack_trace.go	`100.00% <100.00%> (ø)`
errors/stack_trace_js.go	`100.00% <100.00%> (ø)`
js/client_tx.go	`100.00% <100.00%> (ø)`
net/config/config.go	`100.00% <ø> (ø)`
net/host.go	`76.32% <100.00%> (ø)`
net/peer.go	`74.65% <100.00%> (ø)`
... and 11 more

... and 18 files with indirect coverage changes

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1281b6a...52f44ca. Read the comment docs.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

AndrewSisley

Looks really good Keenan! Is really neat considering what it manages to achieve :)

Is a shame we can quite do a pure wasm build, yet, but maybe Chris's C binding work will solve that soon.

I didn't review any code that moved, please let me know if anything was changed in the code that did move.

I have a few questions, but I don't think any answers would cause me to want to block the PR so I'm approving now.

AndrewSisley · 2025-05-05T20:52:12Z

Makefile

 	@$(MAKE) clean:coverage
-
+
+.PHONY: test\:coverage-js


question: Why is this a new command? Would GOOS=js GOARCH=wasm make test:coverage not work?

It doesn't work because not all of the packages are JS / WASM compatible. If we wanted to go that route we'd need to add a bunch of //go:build !js tags to the files in those packages..

AndrewSisley · 2025-05-05T20:56:11Z

cli/utils.go

 	}

-	ctx := identity.WithContext(cmd.Context(), immutable.Some(ident))
+	ctx := acpIdentity.WithContext(cmd.Context(), immutable.Some(ident))


question: Why did you remove the unaliased import instead of the aliased one?

I was seeing a linter warning in my IDE about the same packagebeing imported twice.

AndrewSisley · 2025-05-05T21:17:38Z

tests/bench/acp/acp_local_test.go

 // by the Apache License, Version 2.0, included in the file
 // licenses/APL.txt.

+//go:build !js


question: This is a bit annoying to have to include, why is it required?

I think these can be removed now. I'll double check.

It looks like they are not needed. I've removed all of the build flags from tests/bench.

Nice! Thanks Keenan

AndrewSisley · 2025-05-05T21:20:14Z

tests/clients/js/wrapper.go

+
+//go:build js
+
+package js


question: Does this execute the js client as wasm in a JS runtime?

Yes, everything (including the test framework) is executed in a NodeJS runtime.

Nice! That's really great, thanks Keenan.

AndrewSisley · 2025-05-05T21:22:10Z

tests/integration/mutation/update/crdt/pcounter_test.go

 	test := testUtils.TestCase{
 		Description: "Positive increments of a P Counter with Int type causing overflow behaviour",
+		SupportedClientTypes: immutable.Some([]testUtils.ClientType{
+			// JS client does not support 64 bit integers


question: Why not?

https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Number#number_encoding

Can you add/document this somewhere incase there are others who wonder about this later.

AndrewSisley · 2025-05-05T21:22:34Z

tests/integration/utils.go

 	viewType     ViewType
 	// skipNetworkTests will skip any tests that involve network actions
 	skipNetworkTests = false
+	// skipBackupTests will skip any tests taht involve backup actions


nitpick: Typo taht

AndrewSisley · 2025-05-05T21:23:37Z

tests/integration/client_setup_js.go

+	cliClient = false
+	jsClient = true
+	skipNetworkTests = true
+	skipBackupTests = true


todo: Please document why net and backup are skipped (within this func)

done. Let me know if its a good enough explanation.

The 'why' regarding backup is still a little vague to me. Feel free to keep the comment as-is if you feel like though - I think I'm questioning the design decision as much as the documentation.

I do not see a good reason (other than dev-time priority) to not support importing data in a JS runtime, and can easily imagine that to be very useful.

Exporting to file is harder as you mentioned in another comment, however returning the json could be very useful too (and then let the user decide what they want to do with it, instead of forcing it to file).

I don't remember ever looking at the implementation of import/export, but to me working with files (or any other medium) seems to be a very lightweight bonus option that should be sat on top of the core feature.

I agree it could be useful. I'd prefer to do this in a separate PR since this one is already large. It will likely require changes to the backup API as well.

Yes, completely on onboard with that, and it is a different priority really to this main body of work.

AndrewSisley · 2025-05-05T21:24:38Z

tests/clients/js/wrapper.go

+	panic("not implemented")
+}
+
+func (w *Wrapper) BasicImport(ctx context.Context, filepath string) error {


question: Why aren't Import/Export implemented?

We had a short discussion about it in the standup. This backup API implementation doesn't make much sense in JS environments. There's also technical issues with where the files should be written in a browser.

.github/workflows/test-coverage.yml

Makefile

tests/integration/acp_setup_js.go

shahzadlone

LGTM, thanks for doing the suggested changes and investigating why develop change was sneaking in. Just left a question regarding some acp integration test structure refactoring with js build that was done.

shahzadlone · 2025-05-11T22:07:20Z

tests/integration/acp_setup_js.go

question: I am a bit confused, so stuff was moved from acp.go into acp_setup.go and acp_setup_js.go. And only the acp_setup.go is ignored when building with js build-tag, both other files are included with both js and non-js builds?

Files that end in _js.go automatically have the JS build tag.

What I was wondering was does this mean that when we don't build with js build tag, then the _js.go will still be loaded and is not being ignored, is that expected?

i.e. acp_setup.go is being ignored when we build with js tag so was thinking that if we aren't then the acp_setup_js.go will be ignored similarly (which it is not atm).

It works the same as the _test.go suffix. The files are only included when you use the js build tag.

shahzadlone · 2025-05-11T22:09:14Z

tests/integration/mutation/update/crdt/pcounter_test.go

 	test := testUtils.TestCase{
 		Description: "Positive increments of a P Counter with Int type causing overflow behaviour",
+		SupportedClientTypes: immutable.Some([]testUtils.ClientType{
+			// JS client does not support 64 bit integers


Can you add/document this somewhere incase there are others who wonder about this later.

islamaliev · 2025-06-12T07:53:32Z

Bug bash result: created #3778

## Relevant issue(s) Resolves sourcenetwork#2442 ## Description This PR adds a JS client implementation to DefraDB. Sorry for the messy commit history. I accidentally started this on a previous feature branch. ## Tasks - [x] I made sure the code is well commented, particularly hard-to-understand areas. - [x] I made sure the repository-held documentation is changed accordingly. - [x] I made sure the pull request title adheres to the conventional commit style (the subset used in the project can be found in [tools/configs/chglog/config.yml](tools/configs/chglog/config.yml)). - [x] I made sure to discuss its limitations such as threats to validity, vulnerability to mistake and misuse, robustness to invalidation of assumptions, resource requirements, ... ## How has this been tested? Added integration test client wrapper. Specify the platform(s) on which this was tested: - MacOS

nasdf and others added 16 commits March 17, 2025 12:46

refactor node package for wasm build

fc3026d

fix build errors and failing tests

c723d3e

fix linter errors

1e6d2e8

disable CGO in JS tests

abc93fc

disable AppArmor in js test workflow

b4c2c2c

move js test workflow to new file

55207c8

pr fixes

8dffdba

update AuthorizedAccountClaim comment

8584850

pr fixes

55e8c9f

Merge branch 'develop' into nasdf/feat/wasm-build

c4c5169

tests passing

9d813db

bump goji version

35ba572

Merge branch 'develop' into nasdf/feat/js-client

3aba4da

add js coverage workflow

6655266

comment out failing test

bcc2c8e

fix workflow formatting

173df6e

nasdf added the feature New feature or request label May 2, 2025

nasdf added this to the DefraDB v0.18 milestone May 2, 2025

nasdf self-assigned this May 2, 2025

nasdf commented May 2, 2025

View reviewed changes

nasdf added 7 commits May 2, 2025 11:54

don't use gotestsum for coverage-js Makefile target

4c5d39b

don't use cache with js test coverage job

ccb98f2

get deps before js test job

26cdcb7

disable cgo in js test job

3825e58

Merge branch 'develop' into nasdf/feat/js-client

d237afd

fix deadlock in subscription test

cd5b5fd

include js test in upload coverage job requirements

36d078a

nasdf marked this pull request as ready for review May 5, 2025 20:48

nasdf requested a review from a team May 5, 2025 20:48

AndrewSisley approved these changes May 5, 2025

View reviewed changes

shahzadlone reviewed May 5, 2025

View reviewed changes

.github/workflows/test-coverage.yml Outdated Show resolved Hide resolved

shahzadlone reviewed May 5, 2025

View reviewed changes

Makefile Show resolved Hide resolved

tests/integration/acp_setup_js.go Outdated Show resolved Hide resolved

nasdf added 2 commits May 6, 2025 06:49

pr fixes

e9b6a80

remove build flags from tests/bench

109db69

nasdf requested a review from shahzadlone May 6, 2025 14:01

nasdf and others added 5 commits May 6, 2025 09:09

add temporary ssh workflow step

7432348

remove temp ssh workflow step

cc546c1

Merge branch 'develop' into nasdf/feat/js-client

485d521

update collection interface

70894e0

Merge branch 'develop' into nasdf/feat/js-client

fe17211

shahzadlone reviewed May 11, 2025

View reviewed changes

shahzadlone approved these changes May 11, 2025

View reviewed changes

nasdf added 5 commits May 12, 2025 09:30

updates for collection changes

d3263f4

implement getAllDocIDs

72f0470

Merge branch 'develop' into nasdf/feat/js-client

71d322b

add 64 bit int documentation

daf9fee

fix node js test

52f44ca

nasdf merged commit bb6c439 into sourcenetwork:develop May 12, 2025
44 of 46 checks passed

fredcarle mentioned this pull request Jul 16, 2025

WASM client API #2441

Closed


		//go:build js

		package js

feat: JS client #3658

feat: JS client #3658

Uh oh!

Conversation

nasdf commented May 2, 2025

Relevant issue(s)

Description

Tasks

How has this been tested?

Uh oh!

nasdf May 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

codecov bot commented May 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

AndrewSisley left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

shahzadlone left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

nasdf May 2, 2025 •

edited

Loading

codecov bot commented May 2, 2025 •

edited

Loading