Independent Table Elements #34

aaroncox · 2023-07-23T17:48:07Z

This PR breaks the dependencies between Contract, Table, and TableCursor - which allows developers the opportunity to use each element individually if they choose to do so.

src/contract/table-cursor.ts

dafuga · 2023-07-24T21:50:06Z

src/contract/table-cursor.ts

-     *
-     * @returns A new cursor with updated parameters.
-     */
-    query(query: Query) {


Hmm I guess one side effect of decoupling table and table cursor is that this now feels out of place 🤔

Yeah if we want it back - it's going to need to be a function that translates a TableCursor back into a Table call, and that kinda re-adds that dependency this was seeking to remove.

I'm thinking maybe we add this feature into whatever a future QueryBuilder might be, and leave it off for now.

Sounds good!

dafuga · 2023-07-24T21:51:54Z

src/contract/table-cursor.ts


        if (this.next_key) {
            lower_bound = this.next_key
        }

-        let indexPosition = this.tableParams.index_position || 'primary'
-
-        if (this.indexPositionField) {


I have to admit that I do love the fact that this is no longer needed 😁

dafuga · 2023-07-24T21:53:55Z

src/contract/table-cursor.ts


+        const rows =


This variable assignment does a lot and is a bit difficult to follow so I would probably break it up into different steps.

I gave this a shot, and added some comments.

contract/src/contract/table-cursor.ts

Lines 85 to 100 in ede31bf

// Determine if we need to decode the rows, based on if:

// - json parameter is false, meaning hex data will be returned

// - type parameter is not set, meaning the APIClient will not automatically decode

const requiresDecoding =

this.params.json === false && !(query as API.v1.GetTableRowsParamsTyped).type

// Retrieve the rows from the result, decoding if needed

const rows: RowType[] = requiresDecoding

? result.rows.map((data) =>

Serializer.decode({

data,

abi: this.abi,

type: this.type,

})

)

: result.rows

I'm not sure how much more I think we should split it out, unless we wanted to get rid of the ternary operator, and instead do...

let rows = thing if (decode) { rows = otherThing }

dafuga · 2023-07-24T21:55:45Z

src/contract/table-cursor.ts

    private rowsCount = 0
    private maxRows: number = Number.MAX_SAFE_INTEGER

-    /**


I know that these were far from perfect, but we will still want to have some JSDoc statements here, right?

Yeah absolutely. I just kept seeing so many things changing - that I wanted to redo them all if this was the direction we wanted to go 👍

Co-authored-by: Daniel Fugere <[email protected]>

dafuga · 2023-07-24T21:58:59Z

test/tests/cursor.ts

+
+const mockClient = makeClient('https://eos.greymass.com')
+
+suite('Cursor', () => {


dafuga · 2023-07-24T22:01:51Z

test/tests/table.ts

-            )
-        })
+        // TODO: Discuss and remove? No longer works since query is a Table method
+        // test('should allow you to chain index query statements', async () => {


Yeah, I guess we can just remove this. I personally kind of liked the fact that you could chain query statements like this, but given that the cursor and table classes are now built to be used independently, it no longer really makes sense to have a query method on the cursor.

Removed for now, and left a link to this discussion in the commit.

dafuga · 2023-07-24T22:03:15Z

test/utils/decentium.ts

@@ -0,0 +1,382 @@
+// generated by @greymass/abi2core


Did you mean to commit this? 😄

Yeah the unit tests are using it to test with Struct instances from abi2core.

dafuga · 2023-07-24T22:03:31Z

test/utils/eosio.ts

@@ -0,0 +1,799 @@
+// generated by @greymass/abi2core


dafuga

The more I think about it, the more I realize that this is probably a good change to make! I left you some comments about some minor stuff, but overall the PR looks good! Thanks for working on this!

See: #34 (comment)

Same logic as before, just different syntax.

Resulting logic is the same, just simplified the Math.min() call

aaroncox added 15 commits July 21, 2023 17:21

Prevent serialization if rowType is defined.

ada0cde

Adding failing test

4a9d062

Removing generics to serve as a reset to re-add

4efa4fd

Potential fix?

1924be0

Test showing it working

3ba8c77

TableCursor2 test

bf1a38b

Additional test data

77130b6

Merge

a8c48eb

Generics testing

5f527e2

Moved new TableCursor as default cursor

398e447

Altered Table to use new Cursor

fe46d66

Removed secondary Cursor export

3841446

Updated tests for new Cursor

d7fed28

Reapplied mock data

80ca04f

Reworked Table to no longer require a Contract

9c56fdb

dafuga reviewed Jul 24, 2023

View reviewed changes

src/contract/table-cursor.ts Outdated Show resolved Hide resolved

dafuga reviewed Jul 24, 2023

View reviewed changes

Update src/contract/table-cursor.ts

785a49f

Co-authored-by: Daniel Fugere <[email protected]>

dafuga reviewed Jul 24, 2023

View reviewed changes

test/tests/cursor.ts

const mockClient = makeClient('https://eos.greymass.com')

suite('Cursor', () => {

Copy link

Contributor

dafuga Jul 24, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💯

dafuga reviewed Jul 24, 2023

View reviewed changes

test/utils/eosio.ts Outdated

@@ -0,0 +1,799 @@

// generated by @greymass/abi2core

Copy link

Contributor

dafuga Jul 24, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And this?

dafuga approved these changes Jul 24, 2023

View reviewed changes

aaroncox added 4 commits July 24, 2023 15:09

Comments and splitting logic a bit

ede31bf

Removing old test for now

3e11e8a

See: #34 (comment)

Split out limit/bounds logic, and added comments.

4b7220b

Same logic as before, just different syntax.

Defaulting to MAX_SAFE_INTEGER

39a1f72

Resulting logic is the same, just simplified the Math.min() call

Documentation pass

e3cc543

aaroncox changed the base branch from dev to rowtype-generics-maybe-fix July 24, 2023 22:47

aaroncox changed the base branch from rowtype-generics-maybe-fix to dev July 24, 2023 22:47

aaroncox added 2 commits July 24, 2023 15:59

Remove timeout from ci-test build

515a0c0

Moved structs into better location

3afe6bd

aaroncox merged commit aa3c1f5 into dev Jul 24, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Independent Table Elements #34

Independent Table Elements #34

aaroncox commented Jul 23, 2023

dafuga Jul 24, 2023

aaroncox Jul 24, 2023

dafuga Jul 24, 2023

dafuga Jul 24, 2023

dafuga Jul 24, 2023 •

edited

Loading

aaroncox Jul 24, 2023

dafuga Jul 24, 2023

aaroncox Jul 24, 2023

dafuga Jul 24, 2023

dafuga Jul 24, 2023

aaroncox Jul 24, 2023

dafuga Jul 24, 2023

aaroncox Jul 24, 2023

dafuga Jul 24, 2023

dafuga left a comment

	// Determine if we need to decode the rows, based on if:
	// - json parameter is false, meaning hex data will be returned
	// - type parameter is not set, meaning the APIClient will not automatically decode
	const requiresDecoding =
	this.params.json === false && !(query as API.v1.GetTableRowsParamsTyped).type

	// Retrieve the rows from the result, decoding if needed
	const rows: RowType[] = requiresDecoding
	? result.rows.map((data) =>
	Serializer.decode({
	data,
	abi: this.abi,
	type: this.type,
	})
	)
	: result.rows


		const mockClient = makeClient('https://eos.greymass.com')

		suite('Cursor', () => {

Independent Table Elements #34

Independent Table Elements #34

Conversation

aaroncox commented Jul 23, 2023

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

dafuga Jul 24, 2023 • edited Loading

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

dafuga left a comment

Choose a reason for hiding this comment

dafuga Jul 24, 2023 •

edited

Loading