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

fix(device_info_plus): fix type casting for WASM #3161

Closed
wants to merge 1 commit into from

Conversation

GiacomoPignoni
Copy link

@GiacomoPignoni GiacomoPignoni commented Aug 9, 2024

Description

Fix some types casting on web implementation for WASM compilation

Related Issues

Checklist

  • I read the Contributor Guide and followed the process outlined there for submitting PRs.
  • I titled the PR using Conventional Commits.
  • I did not modify the CHANGELOG.md nor the plugin version in pubspec.yaml files.
  • All existing and new tests are passing.
  • The analyzer (flutter analyze) does not report any problems on my PR.

@vbuberen
Copy link
Collaborator

vbuberen commented Aug 9, 2024

FIX #2966

The mentioned issue is about package_info_plus, while your changes are for device_info_plus.

Could you explain why you did the change in this PR? device_info_plus supports a range of web versions https://github.com/fluttercommunity/plus_plugins/blob/main/packages/device_info_plus/device_info_plus/pubspec.yaml#L40 and works well with both web 0.5.1 and 1.0.0 at the moment.

@GiacomoPignoni
Copy link
Author

I'm sorry,I got confused, I did notice the PR was for package_info_plus:
Still that the package device_info_plus, the one I'm fixing in this PR, has a similar problem when used in web compiled in WASM.

  • The type of deviceMemory came out as a double on WASM
  • The languages field it's a JSArray<JSString> so it needs to pass through toDart

@vbuberen
Copy link
Collaborator

vbuberen commented Aug 9, 2024

Still that the package device_info_plus, the one I'm fixing in this PR, has a similar problem when used in web compiled in WASM.

I would ask you to open the issue first and fill in all the required data, because from just this statement it is not clear in which environment, which versions you used etc. As I mentioned we check WASM compilation as well and everything worked fine so far. So if you found an issue, it would be better to report is properly first, so we could have a constructive discussion about this PR.

@miquelbeltran
Copy link
Member

The changes in this PR are the same as #3254

Let's follow up on that other PR, I will close this one.

In all cases, thanks for the contribution @GiacomoPignoni !

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

Successfully merging this pull request may close these issues.

[Bug]: Wasm illegal cast
3 participants