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

Consider using num instead of double for parameter types #57

Closed
sigmundch opened this issue Jun 27, 2023 · 3 comments · Fixed by #78 or #256
Closed

Consider using num instead of double for parameter types #57

sigmundch opened this issue Jun 27, 2023 · 3 comments · Fixed by #78 or #256

Comments

@sigmundch
Copy link
Member

This is more backwards compatible with types used in dart:html, and hence less work to adapt a call that used to pass num or int arguments.

srujzs added a commit to srujzs/web that referenced this issue Sep 25, 2023
Closes dart-lang#57

Makes it easier to migrate off of dart:html APIs and allows users
to pass integers without having to convert to doubles.
srujzs added a commit to srujzs/web that referenced this issue Sep 25, 2023
Closes dart-lang#57

Makes it easier to migrate off of dart:html APIs and allows users
to pass integers without having to convert to doubles.
srujzs added a commit to srujzs/web that referenced this issue Sep 26, 2023
Closes dart-lang#57

Makes it easier to migrate off of dart:html APIs and allows users
to pass integers without having to convert to doubles.
srujzs added a commit to srujzs/web that referenced this issue Sep 27, 2023
Closes dart-lang#57

Makes it easier to migrate off of dart:html APIs and allows users
to pass integers without having to convert to doubles.
srujzs added a commit to srujzs/web that referenced this issue Sep 27, 2023
Closes dart-lang#57

Makes it easier to migrate off of dart:html APIs and allows users
to pass integers without having to convert to doubles.
srujzs added a commit that referenced this issue Sep 28, 2023
Closes #57

Makes it easier to migrate off of dart:html APIs and allows users
to pass integers without having to convert to doubles.
@srujzs
Copy link
Contributor

srujzs commented Oct 25, 2023

Reopening due to concerns around users accidentally downcasting num in dart2wasm because they may assume dart2wasm has the same number semantics as the JS compilers (it does not).

A compromise here to:

  • Make migration and passing in integer values easier
  • Avoid the accidental downcasting

is to keep the parameter types as num but the return types as double.

cc @eyebrowsoffire

@srujzs srujzs reopened this Oct 25, 2023
@parlough
Copy link
Member

parlough commented Feb 9, 2024

I also think this might be worth reevaluating to some extent. My feeling is that today's Dart developers aren't as familiar with num as they used to be.

Cases passing int literals would keep working with double and I think any remaining changes necessary shouldn't be too bad. The extra consideration might be helpful in the first place.

@srujzs
Copy link
Contributor

srujzs commented Mar 1, 2024

I agree. I want to perhaps include this in the next major release (which we'll certainly have as we tackle other issues). It does mean a little bit of pain for migration, but it should be minimal.

My feeling is that today's Dart developers aren't as familiar with num as they used to be.

Most developers aren't aware of the number differences partly because it derives from a web vs native difference. Now that we have the number difference within web, this will pop up more often.

@srujzs srujzs mentioned this issue Apr 22, 2024
srujzs added a commit to srujzs/web that referenced this issue Jun 17, 2024
Closes dart-lang#57

In cases where the return type is known to be a double
value, prefer double over num so users don't try and
downcast the type, which won't work consistently on
all compilers. In order to do this, _typeReference is
amended to take an extra option 'returnType' which
determines if the `_RawType` is used in a return type
context.
@srujzs srujzs closed this as completed in f6a7d38 Jun 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
3 participants