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

Port advanced Stardust examples to Wasm #1018

Merged
merged 36 commits into from Sep 19, 2022
Merged

Port advanced Stardust examples to Wasm #1018

merged 36 commits into from Sep 19, 2022

Conversation

PhilippGackstatter
Copy link
Contributor

@PhilippGackstatter PhilippGackstatter commented Sep 14, 2022 •
edited

Description of change

Links to any relevant issues

#984

Type of change

Add an x to the boxes that are relevant to your changes.

  • Bug fix (a non-breaking change which fixes an issue)
  • Enhancement (a non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation Fix

How the change has been tested

Run the new examples locally and add them as tests to run in CI.

Change checklist

Add an x to the boxes that are relevant to your changes.

  • I have followed the contribution guidelines for this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes

@PhilippGackstatter PhilippGackstatter added Documentation Improvements or additions to documentation. Excludes from changelog Wasm Related to Wasm bindings. Becomes part of the Wasm changelog labels Sep 14, 2022
@PhilippGackstatter PhilippGackstatter added this to the v0.7 Features milestone Sep 14, 2022
@PhilippGackstatter PhilippGackstatter linked an issue Sep 14, 2022 that may be closed by this pull request
[Task] Port Rust Stardust examples to Wasm #984
Closed
8 tasks
@PhilippGackstatter PhilippGackstatter added the Added A new feature that requires a minor release. Part of "Added" section in changelog label Sep 15, 2022
@PhilippGackstatter PhilippGackstatter marked this pull request as ready for review September 15, 2022 12:33
cycraig
cycraig approved these changes Sep 15, 2022
View reviewed changes
Copy link
Contributor

@cycraig cycraig left a comment

Choose a reason for hiding this comment

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

Thanks for porting all the examples to Typescript. Looks good to me so far, minor comments below.

bindings/wasm/tsconfig.json Outdated Show resolved Hide resolved
bindings/wasm/package.json Outdated Show resolved Hide resolved
bindings/wasm/package.json Outdated Show resolved Hide resolved
bindings/wasm/src/iota/iota_did.rs Outdated Show resolved Hide resolved
bindings/wasm/examples/src/tests/4_delete_did.ts Outdated Show resolved Hide resolved
bindings/wasm/examples/src/1_advanced/0_did_controls_did.ts Show resolved Hide resolved
bindings/wasm/examples/src/1_advanced/3_did_issues_tokens.ts
const companyAddress = Bech32Helper.addressFromBech32(companyAddressBech32, networkName);

// Create a timestamp 24 hours from now.
const tomorrow: number = Math.floor(Date.now() / 1000) + (60 * 60 * 24);
Copy link
Contributor

Choose a reason for hiding this comment

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

Could we use the library's Timestamp and Duration to simplify this? The only problem was the lack of .toUnix due to u64/bigint problems in wasm-bindgen`, but that seems to have been fixed with the latest release? rustwasm/wasm-bindgen#3037

Maybe an issue is needed, if only to expose .toUnix.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, I had it with Timestamp and Duration originally but indeed the lack of toUinx was the reason to use Date.now instead. Would be nice if we could revert this PR completely: #365.
The reason for removal were issues on React Native specifically (#362), is that no longer a problem then?

@PhilippGackstatter PhilippGackstatter merged commit ae0a016 into dev Sep 19, 2022
@PhilippGackstatter PhilippGackstatter deleted the chore/wasm-examples-refactor branch September 19, 2022 14:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@eike-hass eike-hass eike-hass left review comments

@cycraig cycraig cycraig approved these changes

@abdulmth abdulmth abdulmth approved these changes

Assignees
No one assigned
Labels
Added A new feature that requires a minor release. Part of "Added" section in changelog Documentation Improvements or additions to documentation. Excludes from changelog Wasm Related to Wasm bindings. Becomes part of the Wasm changelog
Projects
IOTA Identity - Framework Developments
Status: Done
Milestone
v0.7 Features
Development

Successfully merging this pull request may close these issues.

[Task] Port Rust Stardust examples to Wasm
4 participants
@PhilippGackstatter @eike-hass @cycraig @abdulmth