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

refactor(formats): extract formats into separate packages #1536

Merged
merged 4 commits into from
Mar 21, 2023

Conversation

thekip
Copy link
Collaborator

@thekip thekip commented Mar 20, 2023

Description

Extract catalog formats as separate packages inside lingui monorepo:

  • csv -> @lingui/format-csv
  • mimial -> @lingui/format-json with {style: "minimal"}
  • lingui -> @lingui/format-json with {style: "lingui"} (or rename it to full? TBD)
  • po -> @lingui/format-po
  • po-gettext -> @lingui/format-po-gettext - this one is an edge case. After release, i want to check how many downloads it has and based on that - deprecate it. This one is pretty hard to support with little value and with new api of custom formatters could be simply implemented on the userland.

Second important thing happend here:

I started using unbuild (BTW, which is awesome) for packaging in monorepo. I will be moving towards replacing all build pipelines to the unbuild + yarn workspaces in next iterations. The two main selling point unbuild gives:

  • it automatically builds cjs / mjs / dts without configuration. Just reading our package.json export
  • stubbing modules during development. This resolves issue that something works while dev but don't work when compiled and vice versa and avoid a necessity to configure tsconfig`s paths and teach all tools to work with it.

Migration and deprecations

For the po users, no migration is needed. The @lingui/format-po is listed in the dependencies of @lingui/cli and enabled by default.

For the other formats, the error with helpful message would be thrown, so the users should update theirs config accordingly.

@vercel
Copy link

vercel bot commented Mar 20, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated
js-lingui ✅ Ready (Inspect) Visit Preview 💬 Add your feedback Mar 21, 2023 at 8:52AM (UTC)

@github-actions
Copy link

github-actions bot commented Mar 20, 2023

size-limit report 📦

Path Size
./packages/core/build/esm/index.js 1.57 KB (0%)
./packages/detect-locale/build/esm/index.js 812 B (0%)
./packages/react/build/esm/index.js 1.59 KB (0%)
./packages/remote-loader/build/esm/index.js 7.25 KB (0%)

@codecov
Copy link

codecov bot commented Mar 20, 2023

Codecov Report

Patch coverage: 70.45% and project coverage change: -0.08 ⚠️

Comparison is base (94863c5) 75.19% compared to head (0b29f0f) 75.11%.

❗ Current head 0b29f0f differs from pull request most recent head 284fe4c. Consider uploading reports for the commit 284fe4c to get more accurate results

Additional details and impacted files
@@            Coverage Diff             @@
##             next    #1536      +/-   ##
==========================================
- Coverage   75.19%   75.11%   -0.08%     
==========================================
  Files          79       78       -1     
  Lines        1963     1961       -2     
  Branches      515      511       -4     
==========================================
- Hits         1476     1473       -3     
- Misses        371      376       +5     
+ Partials      116      112       -4     
Impacted Files Coverage Δ
packages/cli/src/api/formats/formatterWrapper.ts 100.00% <ø> (ø)
packages/cli/src/api/utils.ts 61.90% <0.00%> (-4.77%) ⬇️
packages/conf/src/makeConfig.ts 100.00% <ø> (ø)
packages/format-csv/src/csv.ts 80.00% <ø> (ø)
packages/cli/src/api/formats/index.ts 42.85% <14.28%> (+1.19%) ⬆️
packages/format-json/src/json.ts 84.00% <81.81%> (ø)
packages/format-po/src/po.ts 91.66% <88.88%> (ø)
packages/format-po-gettext/src/po-gettext.ts 79.16% <100.00%> (ø)

... and 2 files with indirect coverage changes

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

package.json Outdated Show resolved Hide resolved
packages/cli/package.json Outdated Show resolved Hide resolved
packages/cli/package.json Outdated Show resolved Hide resolved
packages/cli/src/api/catalog.test.ts Show resolved Hide resolved
packages/cli/src/api/formats/index.ts Outdated Show resolved Hide resolved
website/docs/ref/conf.md Outdated Show resolved Hide resolved
website/docs/ref/conf.md Outdated Show resolved Hide resolved
website/docs/ref/conf.md Outdated Show resolved Hide resolved
website/docs/ref/conf.md Outdated Show resolved Hide resolved
tsconfig.json Show resolved Hide resolved
@andrii-bodnar
Copy link
Contributor

@thekip thank you!

I don't really like the idea of creating a separate @lingui/format-po-gettext package just to see the usage statistics. Moreover, the download statistics will not be trustworthy since users can use old versions for years and it will take a lot of time to migrate.

I would suggest here to act in the same way as with the json formatter - control it via the @lingui/format-po formatter configuration flag and isolate the functionality as much as possible. I don't think it will create a lot of maintenance debt

@andrii-bodnar
Copy link
Contributor

unbuild looks interesting, but it also looks not so popular at the moment. We need to be sure that there are no risks in locking on it.

@thekip
Copy link
Collaborator Author

thekip commented Mar 20, 2023

don't really like the idea of creating a separate @lingui/format-po-gettext package just to see the usage statistics. Moreover, the download statistics will not be trustworthy since users can use old versions for years and it will take a lot of time to migrate.

I would suggest here to act in the same way as with the json formatter - control it via the @lingui/format-po formatter configuration flag and isolate the functionality as much as possible. I don't think it will create a lot of maintenance debt

I want to get rid of it anyway. I will not change here as you asked. This module is an edge case implemented for someone. And probably this "someone" already forgot that implemented it, but we still need to maintain it. And maintaing this one is hard, because code is complicated.

This formatter is not recommended anyway because it just doesn't work with lingui properly, only subset of features is supported.

@thekip
Copy link
Collaborator Author

thekip commented Mar 20, 2023

unbuild looks interesting, but it also looks not so popular at the moment. We need to be sure that there are no risks in locking on it.

Every tools i found for library authors are not popular. It's simply because the amount of developers who create libraries vs consume libraries is very diffrent.

unbuild is a part of UnJs ecosystem, which in turn part of Nuxt ecosystem and actively maintained for them.

@andrii-bodnar
Copy link
Contributor

Reference to original PR - #677

I didn't read all the discussions but I don't think that this should be deprecated at all. It can be an opt-in feature (currently, we can accept the fact that it works not with all Lingui features as you said)

@thekip
Copy link
Collaborator Author

thekip commented Mar 20, 2023

@andrii-bodnar we can ask original author to extract this from the Lingui repository and create a separate package.

@andrii-bodnar
Copy link
Contributor

Since it's already in the Lingui repository, we can't just remove a part of the functionality. It should happen carefully. So let's keep it like it is in this PR as a separate package. And then, in one of the future major releases (v5, v6, etc) will think about what to do with that.

@thekip
Copy link
Collaborator Author

thekip commented Mar 20, 2023

The general idea it not maintainng this as a part of a lingui project. Thanks to custom formatters community now can create new formats by their own. All edge case implementation such as po-gettext should be community driven.

@thekip
Copy link
Collaborator Author

thekip commented Mar 20, 2023

Anything else here should be done?

packages/format-csv/README.md Outdated Show resolved Hide resolved
packages/format-csv/package.json Outdated Show resolved Hide resolved
packages/format-po-gettext/README.md Outdated Show resolved Hide resolved
packages/format-po-gettext/package.json Outdated Show resolved Hide resolved
Co-authored-by: Andrii Bodnar <andrii.bodnar@crowdin.com>
@andrii-bodnar andrii-bodnar merged commit 36272c4 into lingui:next Mar 21, 2023
@thekip thekip deleted the refactor/extract-formats branch March 21, 2023 09:22
@andrii-bodnar andrii-bodnar mentioned this pull request Mar 21, 2023
8 tasks
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.

None yet

2 participants