Skip to content

Conversation

@ilonatommy
Copy link
Member

@ilonatommy ilonatommy commented Aug 9, 2023

PR #90145 revealed failures on Hybrid hosted on NodeJS. The differences in behavior should be removed once we update to node v20.

The reason for this PR might be unclear as it would seem better to fix the behavior then to change the tests. We have a few cases here:

  • Differences in the API's responses between hosts are marginal e.g. "Sept" as September abbreviation instead of "Sep" and hardcoding the fix would be easy but adding additional performance overhead (a forest of if-cases) or adding a bunch of static data.
  • Differences in the API's responses are big e.g. first day of the week. For some locales it's hard to define the first day of the week, e.g. see "en-AU" - in dotnet we originally return "Sunday" even though ISO 8601 suggests "Monday". Node and v8/browser are not in line with the decision either. Hardcoding is possible but we would rather inform users what JS function is used to fetch the response:
    | FirstDayOfWeek | `Intl.Locale.prototype.getWeekInfo().firstDay` | zn-CN | Sunday | Monday |

    so that they would be conscious about the "FirstDayOfWeek" being directly bound to the host's answer to that JS function.
  • Differences that are big and wrong. E.g. "bn-IN" AbbreviatedMonthNames returns abbreviated genitive name for browser/v8. Node behaves fine. We are aware of the buggy behavior but due to reasons from previous points we decide to keep uniform and fast preforming code, over fixing the individual host's implementations.

@ghost
Copy link

ghost commented Aug 9, 2023

Tagging subscribers to 'arch-wasm': @lewing
See info in area-owners.md if you want to be subscribed.

Issue Details

PR #90145 revealed failures on Hybrid hosted on NodeJS. The differences in behavior should be removed once we update to node v20.

Author: ilonatommy
Assignees: ilonatommy
Labels:

arch-wasm, area-System.Globalization, area-Build-mono

Milestone: -

@ilonatommy
Copy link
Member Author

/azp run runtime-wasm

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@ilonatommy ilonatommy merged commit 888a7c9 into dotnet:main Aug 9, 2023
radical pushed a commit to radical/runtime that referenced this pull request Sep 1, 2023
radical added a commit that referenced this pull request Sep 5, 2023
* [wasm] Move helix queues to ubuntu 22.04

* Revert #90223 + fix v8 tests.

* CI: update other references for wasm helix images

---------

Co-authored-by: Ilona Tomkowicz <[email protected]>
@ghost ghost locked as resolved and limited conversation to collaborators Sep 8, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants