Migrate lib/src/crypto_subtle.dart to use dart:js_interop
#86
Conversation
koji-1009
force-pushed
the
feat/js_interop
branch
2 times, most recently
from
1e992d3 to
22c5230
Compare
koji-1009
force-pushed
the
feat/js_interop
branch
from
b9c9d83 to
7e2dd45
Compare
There was a problem hiding this comment.
I think this is good enough to ship!
If you want to tweak the test things, feel free. Otherwise, I'll land this, and be happy to accept PRs improving tests.
Writing tests for crypto_subtle.dart actually makes sense. But it's a non-trivial amount of work, and certainly not something we should block this PR on.
| @@ -0,0 +1,186 @@ | |||
| @TestOn('chrome') | |||
There was a problem hiding this comment.
I imagine that the two files for Firefox and Chrome are quite similar.
Can we make this a single file.
Perhaps we can make detectedRuntime to handle different browsers differently.
If we only have a single file it's easier to spot the difference.
| import 'package:webcrypto/src/impl_js/impl_js.dart'; | ||
| import 'package:webcrypto/src/crypto_subtle.dart'; | ||
| import 'package:webcrypto/src/testing/webcrypto/random.dart'; |
There was a problem hiding this comment.
Perhaps we should only be testing:
src/crypto_subtle.dart
And import it with import 'package:webcrypto/src/crypto_subtle.dart as subtle' as we do in impl_js.dart`.
Basically, we could call this crypto_subtle_test.dart, because it contains unit tests that the way we wrap the browsers API in crypto_subtle.dart is as we would expect.
(I don't really care that you import isAllZero that's fine, hehe)
| .having( | ||
| (e) => e.name, | ||
| 'name', | ||
| 'TypeError', | ||
| ) | ||
| .having( | ||
| (e) => e.message, | ||
| 'message', | ||
| contains( | ||
| '''Failed to execute 'generateKey' on 'SubtleCrypto': Algorithm: name: Missing or not a string''', | ||
| ), |
There was a problem hiding this comment.
If we code it this specifically, then it'll never across browsers.
But we could say that we expect a JSDomException with e.name being oneOf('TypeError', 'SyntaxError') -- I'm not sure we should care about the difference.
|
I'll get this out in the next release, feel free to file a PR preparing a release (bump version and update changelog), if you want it sooner. |
|
@jonasfj |
Migrate web crypto api code to
dart:js_interop.I have added
JSto class names, likeJSWindow, to distinguish it from classes that are not related todart:js_interop. But if this is unnecessary, please point it out in review comments.close #83