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

feat(core/runtime): Feature gate ICU #10114

Closed
wants to merge 1 commit into from

Conversation

littledivy
Copy link
Member

Leverages cargo features to feature gate ICU initialization. By default, the feature is enabled but can be disabled with features = ["no_icu"]

Use case: I'm using Deno for embedding purposes and would like to not inlcude ICU data by default into binaries.

@lucacasonato
Copy link
Member

This does not work. If rusty_v8 / v8 are built with ICU support, not supplying ICU info and calling Intl APIs will result in a segfault.

@bnoordhuis
Copy link
Contributor

The V8 team has hinted that ICU may become a required dependency at one point so this is probably not a good long-term solution anyway.

What could work is the "small-icu" approach that Node.js used to take (slimmed down ICU data file that only supports en_US) but they moved away from that after ample discussion: nodejs/node#19214

(I say "they" but I was involved in the decision making around both small-icu and full-icu. In retrospect, I'd say small-icu was a mistake.)

@littledivy
Copy link
Member Author

Fair, will close this PR since it introduces segfaults and might not be good in the long term

@littledivy littledivy closed this Apr 11, 2021
@littledivy littledivy deleted the core/icu_feature branch April 11, 2021 03:39
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