Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Remove Buffer Usage #801

Merged
merged 40 commits into from
Jul 27, 2023
Merged

Remove Buffer Usage #801

merged 40 commits into from
Jul 27, 2023

Conversation

algochoi
Copy link
Contributor

@algochoi algochoi commented Jul 19, 2023 •
edited
Loading

Follow up PR to remove Buffer usage in JS SDK:

Some extra changes:

  • Increase indexer sleep time to fix flaky asa example
  • Re-arrange some imports

Closes #734

@algochoi
Remove buffer usage in favor or Uint8Array and Dataview
b60a57f
@algochoi
Add byteoffset when creating DataView
1cce5ff
@algochoi
Increase Indexer sleep time
31104e7
@algochoi
Add polyfill for base64 decoding and update examples
f3d871a
@algochoi
Update examples to use utility function for Buffer functionality
0289253
@algochoi
Revert smoke test
c161ec2
@algochoi
Add a roundtrip test for sanity check
5c188c5
@algochoi
Fix base64 encoding test
3a6ab1d
@algochoi
Remove buffer in transaction
f6634fc
@algochoi
Remove unnecessary casting
a722245
@algochoi
Merge branch 'remove-buffer-abi' into replace-buffer
736ca01
@algochoi
Unify return type to Uint8Array
c8495f6
@algochoi
Remove buffer usage in source code except client
0c9b49a
@algochoi
Standardize tests and add more utf-8 strings
20861bd
@algochoi
Add hex encoding and string encoding tests
5d86934
@algochoi
Merge branch 'remove-buffer-abi' into replace-buffer
fd9bdf4
@algochoi
Fix some code broken by existing tests
4fe705a
@algochoi
Merge branch '3.0.0' into remove-buffer-abi
209cb4e
@algochoi
Update tests for edge bytes
1437020
@algochoi
Export byte conversion utilities
42f9a12
@algochoi
Fix imports to prefix algosdk
0c5e393
@algochoi
Merge branch 'remove-buffer-abi' into replace-buffer
982e5be
@algochoi
Merge branch '3.0.0' into replace-buffer
56b518d
@algochoi
Resolving merge conflicts
f8230e5
@algochoi
Remove some generated types from stable release
f12c1f6
@algochoi
Fix node tests
f765cd9
@algochoi
Try loadResource in browser
3ddfaec
@algochoi
Update buffer usage in client files
84f73c3
@algochoi
Revert cucumber browser loadResource
3fc5200
@algochoi
Fix browser tests
a84101e
@algochoi algochoi requested a review from a team July 20, 2023 18:46
algochoi
algochoi commented Jul 20, 2023
View reviewed changes
examples/asa.ts
@@ -49,7 +49,7 @@ async function main() {
console.log(`Asset Params: ${JSON.stringify(assetInfo.params)}`);
// example: ASSET_INFO

await new Promise((f) => setTimeout(f, 2000)); // sleep to ensure indexer is caught up
await new Promise((f) => setTimeout(f, 5000)); // sleep to ensure indexer is caught up
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This looks frequently flaky on CI, but I haven't seen it fail locally 👀

@algochoi algochoi mentioned this pull request Jul 20, 2023
Merged
@algochoi algochoi marked this pull request as ready for review July 26, 2023 13:14
Eric-Warehime
Eric-Warehime reviewed Jul 26, 2023
View reviewed changes
src/encoding/binarydata.ts Outdated Show resolved Hide resolved
src/client/v2/algod/models/types.ts
@@ -3,10 +3,10 @@
*/
Copy link
Contributor

Choose a reason for hiding this comment

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

Needs algorand/generator#65 merged.

src/transaction.ts Outdated Show resolved Hide resolved
jasonpaulos
jasonpaulos reviewed Jul 26, 2023
View reviewed changes
Copy link
Contributor

@jasonpaulos jasonpaulos left a comment

Choose a reason for hiding this comment

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

I think we'd also want to remove the dependency on the 3rd party buffer package to consider #734 complete. Do you plan to do that in this PR?

tests/cucumber/steps/index.js Show resolved Hide resolved
jasonpaulos
jasonpaulos reviewed Jul 27, 2023
View reviewed changes
Copy link
Contributor

@jasonpaulos jasonpaulos left a comment

Choose a reason for hiding this comment

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

Will approve once the loadResourceAsJson issue is resolved

tests/cucumber/browser/test.js Outdated Show resolved Hide resolved
tests/cucumber/steps/steps.js Outdated Show resolved Hide resolved
@algochoi algochoi merged commit b09d616 into 3.0.0 Jul 27, 2023
@algochoi algochoi deleted the replace-buffer branch July 27, 2023 19:21
@algochoi algochoi mentioned this pull request Aug 1, 2023
Closed
@jasonpaulos jasonpaulos mentioned this pull request Aug 21, 2024
Closed
@onetechnical onetechnical mentioned this pull request Sep 12, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Eric-Warehime Eric-Warehime Eric-Warehime approved these changes

@jasonpaulos jasonpaulos jasonpaulos approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@algochoi @jasonpaulos @Eric-Warehime