pc-dart icon indicating copy to clipboard operation
pc-dart copied to clipboard

add dart2wasm support

Open konsultaner opened this issue 1 year ago • 9 comments

fixes #221

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

konsultaner avatar Feb 27 '24 11:02 konsultaner

This is a blocker at the moment:

~~https://github.com/dart-lang/sdk/issues/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 avatar Feb 27 '24 14:02 konsultaner

@Ephenodrom Do you see any issues with this?

konsultaner avatar Mar 15 '24 12:03 konsultaner

@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 avatar Mar 19 '24 07:03 Ephenodrom

@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

Ephenodrom avatar Mar 19 '24 08:03 Ephenodrom

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 avatar Mar 19 '24 10:03 konsultaner

@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.

konsultaner avatar Mar 19 '24 11:03 konsultaner

@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?

Ephenodrom avatar Mar 25 '24 21:03 Ephenodrom

@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 avatar Mar 26 '24 04:03 konsultaner

@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?

konsultaner avatar Mar 26 '24 21:03 konsultaner

@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>

Ephenodrom avatar Apr 01 '24 13:04 Ephenodrom

@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

konsultaner avatar Apr 03 '24 08:04 konsultaner

@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.

Ephenodrom avatar Apr 03 '24 20:04 Ephenodrom

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

Ephenodrom avatar Apr 05 '24 11:04 Ephenodrom

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

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

sgehrman avatar Oct 22 '24 20:10 sgehrman

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

konsultaner avatar Nov 04 '24 11:11 konsultaner