Optionally build with icu

[ghost]

What is the problem this feature will solve?

I'd like to use a node module that relies on on the Intl global, but the Intl global is not available. I know full icu can be rather large, so the small variant might be a good default. Node itself defaults to providing icu since 14, so it is now more likely to be depended upon.

I still haven't grasped how to build this project properly yet though, so I'm not quite sure how to fix it mysef.

What is the feature you are proposing to solve the problem?

Provide alternative libnode built with --with-intl=small-icu

What alternatives have you considered?

none

[ghost]

Found another issue with the https://github.com/colinhacks/zod library.

SyntaxError: Invalid regular expression: /^(\p{Extended_Pictographic}|\p{Emoji_Component})+$/: Invalid property name

Because in code it looks like this in the code: const emojiRegex = /^(\p{Extended_Pictographic}|\p{Emoji_Component})+$/u;

Note the u

[staltz]

I still haven't grasped how to build this project properly yet though, so I'm not quite sure how to fix it mysef.

Check BUILDING.md and if that's not sufficient, let me know (in details) why.

SyntaxError: Invalid regular expression: /^(\p{Extended_Pictographic}|\p{Emoji_Component})+$/: Invalid property name

I have also stumbled into this problem in the past and it's solvable with a hard-coding approach like these:

• https://github.com/staltz/unicode-word-regex/
• https://github.com/staltz/urlpattern-polyfill-no-unicode

[ghost]

I thought i had written my initial problem with the Intl date time stuff, but it somehow didn't end up in the text.

Note that my problems are with third party modules, not code of mine, so these modules won't help in that case.

As far as building goes here are some of the problems encountered so far.

1. https://github.com/nodejs-mobile/nodejs-mobile/blob/main/doc_mobile/BUILDING.md#building-the-android-library-on-linux-or-macos
   This step mentions checking out a separate branch, but that branch looks pretty out of date, so I'm confused about the interaction there.

2. It seems not to respect the CC/CXX variables, but rather expects gcc to exist.

   As seen here: https://github.com/nodejs-mobile/nodejs-mobile/blob/main/android-configure#L61

   I ran into what seems to be an issue in v8 brought about by gcc 13 changes. V8 folks are going to fix it, but I didn't follow through to see if the change will be backported.

   I was hoping it could fully use the clang prebuilt toolchain as provided by ndk in the meantime at least.

3. Seems to require a fair amount of dependencies on the host system.

   It might be valuable to have a Dockerfile to use a known stable compiler and "host" tools.

[ghost]

I was able to successfully get it to build by adding some missing #include <cstdint> to

• deps/v8/src/base/logging.h
• deps/v8/src/base/macros.h
• deps/v8/src/inspector/v8-string-conversions.h

I'm suprised these changes weren't already backported to v16, but maybe they will be in the future. Here's one of them for reference: https://chromium-review.googlesource.com/c/v8/v8/+/3934140 . It is possible that one those edits i made was unnecessary or that there is another patchset I haven't found that covers the other file.

When building with icu (small-icu), it seems like icu might not be cleaned properly when choosing x (for all arches). It seemed to attempt to link a x86 (or x86_64 i can't remember) against an arm build.

If I get a chance to replicate that I'll let you know.

[ghost]

As far as actually running the build process goes, the instructions say to check out node-js-mobile branch, but maybe that's just a leftover from the previous setup? So far i've been able to build from the main branch.

[keithlayne]

I'd love to be able to use a build (for android) with libicu support, but...seems painful. @jrobeson are you using that build? Just building locally? Maintaining that for my org sounds pretty painful, but if someone maintained a fork I'd probably try it.

I currently use this gnarly snippet as a weak workaround (for reference and in case it's useful to anyone who lands here - YMMV):

// polyfills for nodejs-mobile
global.IntlPolyfill = require('intl/lib/core');
require('intl/locale-data/jsonp/en');
global.Intl = global.IntlPolyfill;
global.IntlPolyfill.__applyLocaleSensitivePrototypes();
require('date-time-format-timezone/build/src/date-time-format-timezone-golden-zones-no-locale');

[ghost]

it didnt' feel that painful. and you probably wouldn't run into that #include <cstdint> issue i ran into if you weren't using gcc 13 like I am. I just changed it to use the small icu in the configure script and it was fine.

The #include <cstdint> will likely not be a problem when the included node is updated to any newer version.

[ghost]

So shouild small_icu be used by default? be provided as a separate build?

[staltz]

If anyone has built nodejs-mobile with icu (of any kind) enabled, what is the final size of the compiled .so?

[ghost]

the total size for arm64 libnode is 60M (that's the only one i've built). The stripped and release mode version is 48M.

[staltz]

That's not too bad, just 10MB more.

The new Node.js Mobile v18 may be approx 60MB with icu=none, so the baseline is going up.

But anyway, 10MB for icu=small is not too bad and I can consider building for both cases and devs can pick which one they want.

[staltz]

I wonder if small ICU may give us also Debugger support.

[jadejr]

@staltz : That's what the maintainer of capacitor-nodejs thinks: hampoelz/Capacitor-NodeJS#23 (comment) What would it take to offer an alternative build with icu? After some experience I do know that it works fine in with the small icu in practice, so it's all about the size. I only have to worry about english, so small is fine for me, but perhaps not others. If more and more libs are likely to depend on icu, then maybe the default should just be small icu even if it is bigger.