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

[wasm] Don't create hybrid .dat for WASM. #536

Conversation

ilonatommy
Copy link
Member

Connected PRs:
dotnet/sdk#45412
dotnet/runtime#110526
dotnet/runtime#110534
dotnet/runtime#110567

In the process of disabling HybridGlobalization feature from WASM, we should remove the icu-transport hybrid package. It fixes the error from the comment:
dotnet/runtime#110567 (comment)

Changes:

  • delete the hybrid .json file from filters
  • skip building icudt_hybrid.dat for WASM but keep it for iOS

@ilonatommy ilonatommy self-assigned this Dec 11, 2024
@akoeplinger akoeplinger merged commit 9bd44b8 into dotnet:dotnet/main Dec 11, 2024
7 checks passed
@akoeplinger
Copy link
Member

Actually I think we can remove it from iOS as well. It uses Hybrid Globalization by default now, which in the iOS case means it only uses ICU from the system instead of our own fork.

@akoeplinger
Copy link
Member

Ah, forgot that I even disabled building this repo for iOS so it's definitely not needed anymore :)

@ilonatommy
Copy link
Member Author

Actually I think we can remove it from iOS as well. It uses Hybrid Globalization by default now, which in the iOS case means it only uses ICU from the system instead of our own fork.

but that feature can be disabled and iOS can run with HybridGlobalization=false if users don't agree with its side effects. Why would we stop shipping ICU for them?

@akoeplinger
Copy link
Member

We've removed that option in .net 9, setting it to false does nothing for iOS anymore.

@akoeplinger
Copy link
Member

to be clear: the side-effects for iOS aren't as dramatic since it uses the same ICU data that Apple ships with iOS, basically like any other native app would

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.

3 participants