Thank you for your PR!

When contributing to Jetpack, we have a few suggestions that can help us test and review your patch:

• ✅ Include a description of your PR changes.
  
• 🔴 Add a "[Status]" label (In Progress, Needs Review, ...).
  
• 🔴 Add a "[Type]" label (Bug, Enhancement, Janitorial, Task).
  
• ✅ Add testing instructions.
  
• ✅ Specify whether this PR includes any changes to data or privacy.
  
• ✅ Add changelog entries to affected projects
  

This comment will be updated as you work on your PR and make changes. If you think that some of those checks are not needed for your PR, please explain why you think so. Thanks for cooperation 🤖

Follow this PR Review Process:

1. Ensure all required checks appearing at the bottom of this PR are passing.
2. Make sure to test your changes on all platforms that it applies to. You're responsible for the quality of the code you ship.
3. You can use GitHub's Reviewers functionality to request a review.
4. When it's reviewed and merged, you will be pinged in Slack to deploy the changes to WordPress.com simple once the build is done.

If you have questions about anything, reach out in #jetpack-developers for guidance!

Code Coverage Summary

Cannot generate coverage summary while tests are failing. 🤐

Please fix the tests, or re-run the Code coverage job if it was something being flaky.

Full summary · PHP report · JS report

Instead of moving everything to i18n-calypso, would it be an option to do the opposite and keep currency formatting i18n-calypso free, so we can continue to rely on Core translations (and thus core language packs and the existing translation process and workflow used by our i18n team and our paid translators) for the different Jetpack plugins developed in the Jetpack monorepo?

Hey @jeherve . I intend to deprecate the format-currency package completely in favor of a centralised number-formatters component. This was part of a PT: pxLjZ-9Ji-p2 and format-currency is no longer used in wp-calypso. So the package is effectively obsolete right now, ready to be removed/deleted. There is a PR to remove it from the monorepo: Automattic/wp-calypso#100559

Now Jetpack remains to follow along the same path. It's not only format-currency (that's also been sitting in legacy/v1 in JP for years), but also the format-number utility. We want everything to go through a single package that carries optimisations and ensures consistent/unified number localisation and rendering. We are targeting Jetpack and Landpack repos as part of another PT: pxLjZ-9ME-p2

You can see the second milestone there is:

Replace usage of format-currency with i18n-calyspo. If this fails, consider creating a number-formatters package (longer path)

So what you are asking is the "longer path" - to create a separate number-formatters package. Are you absolutely categorical/against using i18n-calypso in Jetpack? I'd personally rather we (no, I don't really carry an opinion) One option is to bring i18n-calypso if there are no technical blockers and consider migrating to a single package thereafter (something that could/should be done to both wp-calypso and Jetpack). But I'm also happy to start putting together a number-formatters package with own locale/geo-location state.

Quoting myself:

We want everything to go through a single package that carries optimisations and ensures consistent/unified number localisation and rendering.

We've done several refactors in i18n-calypso with the core components living in a number-formatters folder - with some abstractions on top exported by the package. Creating a separate package will effectively extract both (the core/raw number-formatters handlers) and the top abstractions into a separate package.

We want everything to go through a single package that carries optimisations and ensures consistent/unified number localisation and rendering.

That's fine, that sounds like a nice goal! We would definitely all benefit from an up-to-date, maintained, central solution we can all use.

Does that single package have to be i18n-calypso though?

We've tried very hard to stay away from i18n-calypso in this monorepo until now, because of the translation issues it brings. Namely it is not compatible with GlotPress, which is used to generate all translations for the plugins in this repo. GlotPress is not aware of the translate functions used by i18n-calypso.

We've also discussed about moving away from i18n-calypso in favor of @wordpress/i18n in the Calypso repo as well, but I know this would be a big project. That's most likely why it hasn't happened yet.
Here is a recent conversation about that: p9oQ9f-2Bq-p2#comment-3406

If the number-formatters functions you've been working on do not deeply rely on core i18n-calypso functionality, it sees to me like having a number-formatters package that we can use today would benefit everyone.

Thanks @jeherve

Does that single package have to be i18n-calypso though?

It doesn't, although it does feel to me that we have a few too many decentralised components for i18n. I don't have a strong opinion other than that I'd prefer one vs many down the line. JP having been sitting on v1 format-currency all this time adds more to that concern I think.

p.s. do we have a similar system/concept in Jetpack to wp-calypso for packages, tag and publish to NPM, etc? Would your preference be to host a number-formatters in Jetpack, if so? 🤔

I strongly agree with @jeherve: We've long avoided using i18n-calypso in the Jetpack monorepo because translations done through that package are not picked up by GlotPress, which is the mechanism powering translate.wordpress.org. To work with GlotPress, i18n needs to be done with @wordpress/i18n (or something else that uses same-named underscore functions with identical semantics, but why reinvent everything?).

A separate package for number and currency formatting would be fine with us, as long as any i18n it does gets picked up properly using GlotPress (meaning that it probably needs to either use @wordpress/i18n or rely on the caller to supply any strings). We don't want it to be part of i18n-calypso, as that brings too much risk that someone would wind up also using i18n-calypso's translation infrastructure rather than @wordpress/i18n.

p.s. do we have a similar system/concept in Jetpack to wp-calypso for packages, tag and publish to NPM, etc?

We do! And, IMNSHO, we have better infrastructure around releasing and deploying. 🙂

Each project under projects/js-packages/ is a JavaScript package. Several are automatically deployed to NPM, while others are currently only available within the monorepo.

On the other hand, we haven't done too well yet with setting up TypeScript packages correctly for publishing in a way friendly to reusers (but #41536 is getting there for @automattic/jetpack-components). Setting up more projects as good examples would be useful.

Would your preference be to host a number-formatters in Jetpack, if so? 🤔

If you want to develop it here, we'd be happy to host it. 🙂

Thanks @anomiex
Sounds good. Happy to build it here too. It might help avoid the sync issue seen with format-currency.

On the other hand, we haven't done too well yet with setting up TypeScript packages correctly for publishing in a way friendly to reusers

I hope this isn't more concerning than it sounds 😁

On the other hand, we haven't done too well yet with setting up TypeScript packages correctly for publishing in a way friendly to reusers

I hope this isn't more concerning than it sounds 😁

Mostly it means that many of our packages offer .ts files rather than .js + .d.ts, so reusers may need to configure their build toolchain to compile the node_modules files.

Also a bunch of our packages probably use "moduleResolution": "bundler", so reusers that aren't also using that may run into issues there too (see https://www.typescriptlang.org/docs/handbook/modules/theory.html#module-resolution-for-libraries).

On the other hand, we haven't done too well yet with setting up TypeScript packages correctly for publishing in a way friendly to reusers

I hope this isn't more concerning than it sounds 😁

Mostly it means that many of our packages offer .ts files rather than .js + .d.ts, so reusers may need to configure their build toolchain to compile the node_modules files.

Also a bunch of our packages probably use "moduleResolution": "bundler", so reusers that aren't also using that may run into issues there too (see https://www.typescriptlang.org/docs/handbook/modules/theory.html#module-resolution-for-libraries).

Ok. Thanks. Do you know off-hand if I will need to worry about that with consuming in wp-calypso (so being the main reuser)?

When putting together your new project, you can do it right from the start to avoid there being any problems. There's nothing in the monorepo that will stop you, it's only that people didn't do it when setting up other JS packages because, within the monorepo, we have the build toolchains set up to compile the TS already.

If you decide to go ahead with a js-package in the Jetpack monorepo, I'll be happy to help get it set up. 🙂

When putting together your new project, you can do it right from the start to avoid there being any problems. There's nothing in the monorepo that will stop you, it's only that people didn't do it when setting up other JS packages because, within the monorepo, we have the build toolchains set up to compile the TS already.

If you decide to go ahead with a js-package in the Jetpack monorepo, I'll be happy to help get it set up. 🙂

Thanks @anomiex! I appreciate your offer to help. We had a bit of a chat with @dlind1 in Slack about this, and we will make an attempt to go with Jetpack. I think it might help us, considering we don't stumble nto any major blockers / maintenance hurdles. You can see our reasoning and some concerns here: p1742206025742309/1742141248.562769-slack-C74C3THK7

I'll tag you in a PR to provision the new package. Hopefully you can help us clarify how updates/maintenance will happen, testing changes, also testing them in wp-calypso, etc.

Hopefully you can help us clarify how updates/maintenance will happen,

Releases to NPM generally happen automatically whenever a plugin in the monorepo that uses the package (directly or indirectly) is released. So if it winds up being used in Jetpack-the-plugin, that'll generally be at least weekly due to the weekly Jetpack releases for Atomic. There are also instructions for releasing the package directly if needed (PCYsg-Np7-p2).

Maintenance of the code is mainly up to whichever developers own the code, I can't really speak to that. If no devs are specifically paying attention, it'll still get some incidental updates as we bump dependencies via Renovate, update eslint configs, and so on.

testing changes,

Our main method of JS testing in the monorepo is unit testing with jest. Although if you really want (and will maintain it) you could set up something else; as far as monorepo tooling is concerned, the package just needs to have a composer.json file with .scripts.test-js (and ideally .scripts.test-coverage) defined.

Beyond that we don't have much existing. We have some in-monorepo E2Es for plugins, so you might test the package-as-exposed-by-a-plugin that way. The @group jetpack-wpcom-integration Calypso E2Es get run post-merge against WoA and before the continuous deployments to Simple, which could indirectly test the package that way too.

also testing them in wp-calypso,

Beyond the E2Es mentioned above, we don't have anything set up for this. For manual testing I suppose you'd have to use yarn's equivalent of npm link if it has one.

If you'd rather stick with development in the Calypso repo, we can work with that too. It sounds like the main things to keep in mind with that are sticking with @wordpress/i18n if there's any i18n, and here in the Jetpack monorepo we might want to configure Renovate to be more proactive in updating the package (versus now where on its own it pretty much does Gutenberg packages and one or two others once a month, and relies on people manually triggering stuff otherwise).

Thanks @anomiex ! That's very useful info.

Our main method of JS testing in the monorepo is unit testing with jest

Yes, we have unit tests.

Beyond the E2Es mentioned above, we don't have anything set up for this. For manual testing I suppose you'd have to use yarn's equivalent of npm link if it has one.

That's gold. yarn link should in theory cover what we've been concerned about. So if the process would be like:

1. pull and build the JP monorepo / watch mode
2. link jetpack-number-formatters in wp-calypso
3. make changes in JP and confirm in wp-calypso

I think it should suffice for a Calypso dev wanting to make some changes to the module. I also hope it won't be a common case beyond the initial release - these components are already fairly stable.

If you'd rather stick with development in the Calypso repo, we can work with that too.

I think for now we'll stick (or start) with Jetpack and see how it goes. We don't expect much development on this package beyond the initial release. Well, in theory! I will make sure to add detailed instructions on developing with both repos once I get the handle on it.

Thank you for the generous insight! :-)

Closing. Thanks for the input/discussion