Skip to content

Conversation

srujzs
Copy link
Contributor

@srujzs srujzs commented Sep 25, 2023

Closes #77

typeofEquals now takes a String and returns a bool, and instanceOfString is the canonical replacement for isInstanceOfDomType. Removes now unneeded test as well. Updates min SDK version.

@srujzs srujzs requested a review from sigmundch September 25, 2023 23:48
@srujzs
Copy link
Contributor Author

srujzs commented Sep 25, 2023

This will have to wait until we publish a Dart dev version >= this minimum.

@srujzs srujzs force-pushed the refactordartjsinterophelpers branch from afd66d2 to 53fa83c Compare September 26, 2023 16:27
Copy link
Member

@sigmundch sigmundch left a comment

Choose a reason for hiding this comment

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

Overall looks good, but I'd also be OK keeping a test like the one deleted in helpers_test. That's because, while technically the instnaceOfString should be tested by the SDK, there is still value in having a test here that ensures that it works for package:web types.

@srujzs srujzs force-pushed the refactordartjsinterophelpers branch from 53fa83c to 4c69a99 Compare September 26, 2023 19:12
@srujzs
Copy link
Contributor Author

srujzs commented Sep 26, 2023

Overall looks good, but I'd also be OK keeping a test like the one deleted in helpers_test. That's because, while technically the instnaceOfString should be tested by the SDK, there is still value in having a test here that ensures that it works for package:web types.

Fair point, added it back and refactored to account for instanceOfString.

Copy link
Member

@sigmundch sigmundch left a comment

Choose a reason for hiding this comment

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

Thanks! 👍

@srujzs srujzs force-pushed the refactordartjsinterophelpers branch 9 times, most recently from d6ffa98 to 5d79a7f Compare September 27, 2023 20:27
Closes dart-lang#77

typeofEquals now takes a String and returns a bool, and
instanceOfString is the canonical replacement for isInstanceOfDomType.
Updates min SDK version.
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.

Refactor use of typeofEquals and instanceof and remove isInstanceOfDomType

3 participants