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.

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

add dart2wasm support #226

Merged
merged 2 commits into from
Apr 3, 2024
Merged

add dart2wasm support #226

merged 2 commits into from
Apr 3, 2024

Conversation

konsultaner
Copy link
Contributor

@konsultaner konsultaner commented Feb 27, 2024

fixes #221

  • Random.secure() needs to be fixed by the sdk team

@konsultaner konsultaner marked this pull request as draft February 27, 2024 13:01
@konsultaner
Copy link
Contributor Author

konsultaner commented Feb 27, 2024

This is a blocker at the moment:

dart-lang/sdk#55031

Which has been fixed. This should work with the next Version of dart. For non wasm compilition it should already work.

So it could be merged in my eyes?

@konsultaner konsultaner marked this pull request as ready for review February 27, 2024 14:29
@konsultaner
Copy link
Contributor Author

@Ephenodrom Do you see any issues with this?

@Ephenodrom
Copy link
Contributor

@konsultaner I am not very familiar with Dart / Flutter on web, due to i only use it on CLI or for apps running on different operating systems, therefore my knowledge is limited but I see no problem with the PR. I will check the unit tests and see if they are good.

@Ephenodrom
Copy link
Contributor

@konsultaner There are several tests that fail with the following error : "Failed to load "test/stream/eax_test.dart": The "buf" argument must be an instance of ArrayBuffer or ArrayBufferView. Received an instance of Array".

Please take a look. The tests are run with "dart run test -p node" and use the latest dart version 3.3.1

@konsultaner
Copy link
Contributor Author

The "buf" argument must be an instance of ArrayBuffer or ArrayBufferView. Received an instance of Array

@Ephenodrom I'm on it. I forgot to test it with node. seems a bit tricky, but I know why the tests fail.

@konsultaner
Copy link
Contributor Author

@Ephenodrom I fixed it. Could you try again?

I just also want to notice that you might want to change dart pub run to just dart run in your CI script since it is depricated. if the next version of dart it released you might also want to add a test with --platform chrome -c dart2wasm to also validate the wasm support. For some reason --platform node -c dart2wasm is not supported yet. I guess this is due to a missing wasm-gc support in older node versions. I think the current latest node version should supprt it.

@Ephenodrom
Copy link
Contributor

@konsultaner This looks good so far. I already merged the PR in the origin repo.

When I run the test for the node environment via "dart run test -p node ", there are some tests that fail. Is this intended at the moment?

@konsultaner
Copy link
Contributor Author

konsultaner commented Mar 26, 2024

@Ephenodrom no it is not intended. It should not fail at all. Thats strange. All tests were successful when I tested it locally. Ill have another look at it.

Could you run the github Actions again?

@konsultaner
Copy link
Contributor Author

@Ephenodrom I checked everything again and ran dart test -p node on a node v16.20.1 with dart on 3.3.1. That resulted in:

image

What errors do you get?

@Ephenodrom
Copy link
Contributor

@konsultaner
Ok strange, I tried it with node version v20.6.1 and the latest version v21.7.1.

I receive for example the following error :

00:28 +119 -1: loading test/key_generators/rsa_key_generator_test.dart [E]
  Failed to load "test/key_generators/rsa_key_generator_test.dart": Value of "this" must be of type nullish or must be the global object
  org-dartlang-sdk:///lib/_internal/js_runtime/lib/js_string.dart 202:5   global.cryptoThisCheck
  org-dartlang-sdk:///lib/_internal/js_runtime/lib/math_patch.dart 253:9  _JSSecureRandom._JSSecureRandom
  org-dartlang-sdk:///lib/_internal/js_runtime/lib/math_patch.dart 251:3  <fn>
  org-dartlang-sdk:///lib/_internal/js_runtime/lib/math_patch.dart 74:30  PlatformWeb.PlatformWeb
  package:pointycastle/src/platform_check/web.dart 15:3                   <fn>
  package:pointycastle/src/platform_check/web.dart 65:39                  _generationTests.<fn>.<fn>.<fn>

@konsultaner
Copy link
Contributor Author

konsultaner commented Apr 3, 2024

@Ephenodrom I tried it again on node 21.7.1, dart 3.1.1 on linux with the command:

 dart run test -p node

And the same result, all tests run perfectly.

image

Could you please start the workflow to have the github ci test it too?

image

@Ephenodrom
Copy link
Contributor

@konsultaner The checks look good so far. I have contacted someone from bouncycastle to sync the repository, if this is done I will create a new release on pub.dev. I keep you updated.

@hubot hubot merged commit 4216a28 into bcgit:master Apr 3, 2024
3 checks passed
@Ephenodrom
Copy link
Contributor

@konsultaner The changes are now live on pub.dev with version 3.8.0. Thank you very much for the PR.

@sgehrman
Copy link

It's including the js package, so it's not WASM compatible. Please fix.

dart:js_interop, which replaces package:js and dart:js

@konsultaner
Copy link
Contributor Author

@sgehrman I can't find either of the old packages in the current code. Please link the code, so I can fix it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support dart2wasm
4 participants