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.

Sign up for GitHub

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

Jump to bottom

refactor(extractor)!: don't use intermediate catalogs #1358

Merged
merged 1 commit into from
Feb 9, 2023
Merged

refactor(extractor)!: don't use intermediate catalogs #1358

merged 1 commit into from
Feb 9, 2023

Conversation

thekip
Copy link
Collaborator

@thekip thekip commented Jan 23, 2023 •
edited

Faster, safer, less code

  • add TS typings
  • remove storing to intermediate catalogs and following traversing fs to build final catalog
  • introduce onMessageExtracted callback in babel-plugin options
  • fix unit tests on the babel plugin side
  • fix test on the extractor side

BREAKING CHANGES:

  • The public type of extractor was changed.
- extract(filename: string, targetDir: string, options?: ExtractOptions): Promise<void> | void
+ extract(filename: string, onMessageExtracted: (msg: ExtractedMessage) => void, options?: ExtractOptions): Promise<void> | void

This should affect only small amount of users, only those who use custom extractors.

  • Also intermidiate catalogs no longer created, but i'm sure no one is using it.

@vercel
Copy link

vercel bot commented Jan 23, 2023 •
edited

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 Feb 9, 2023 at 11:25AM (UTC)

@github-actions
Copy link

github-actions bot commented Jan 23, 2023 •
edited

size-limit report 📦

Path Size
./packages/core/build/esm/index.js 1.76 KB (0%)
./packages/detect-locale/build/esm/index.js 812 B (0%)
./packages/react/build/esm/index.js 1.79 KB (0%)
./packages/remote-loader/build/esm/index.js 7.29 KB (0%)

@thekip thekip mentioned this pull request Jan 23, 2023
Closed
8 tasks
thekip
thekip commented Jan 23, 2023
View reviewed changes
packages/babel-plugin-extract-messages/test/fixtures/jsx-without-macros.js Show resolved Hide resolved
packages/cli/src/api/catalog.ts Outdated Show resolved Hide resolved
packages/conf/index.d.ts Outdated Show resolved Hide resolved
@thekip thekip marked this pull request as ready for review January 23, 2023 19:55
@thekip thekip force-pushed the refactor/extractor branch 2 times, most recently from bb516ac to d3ac796 Compare January 24, 2023 08:16
@thekip thekip requested a review from Martin005 January 24, 2023 08:18
@thekip thekip mentioned this pull request Jan 25, 2023
Merged
5 tasks
@thekip
Copy link
Collaborator Author

thekip commented Jan 25, 2023

It's probably better to merge it to the "next" branch, or create a new one for the upcomming release.

@andrii-bodnar
Copy link
Contributor

@thekip please assign yourself all the related issues so it will be easier to recognize what issues actually are in progress

@andrii-bodnar
Copy link
Contributor

@Martin005 @thekip from my perspective, we probably need to create a fresh copy of the next branch from the main for such breaking changes and then move forward with pre-releases from the next branch until it is ready for official release and merge to the main.

In this case, the main and v3 will not be blocked for the minor and patch releases.

I'm going to investigate the possible flow in more detail.

@thekip
Copy link
Collaborator Author

thekip commented Feb 8, 2023

rebased

Martin005
Martin005 reviewed Feb 8, 2023
View reviewed changes
packages/cli/src/api/formats/csv.ts Show resolved Hide resolved
packages/cli/src/lingui-extract.ts Outdated
@@ -26,6 +26,7 @@ export default async function command(
config: LinguiConfig,
options: Partial<CliExtractOptions>
): Promise<boolean> {
// todo validate and delete
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please remove this comment if it is outdated.

packages/cli/src/api/catalog.ts Outdated Show resolved Hide resolved
@Martin005 Martin005 changed the base branch from main to next February 8, 2023 15:42
@thekip
Copy link
Collaborator Author

thekip commented Feb 8, 2023

Done

@thekip thekip requested a review from Martin005 February 8, 2023 17:28
@thekip
Copy link
Collaborator Author

thekip commented Feb 8, 2023

Also rebased again onto next branch

@andrii-bodnar andrii-bodnar merged commit d4a423d into lingui:next Feb 9, 2023
@andrii-bodnar andrii-bodnar added this to the v4 milestone Feb 13, 2023
This was referenced Feb 13, 2023
Closed
Merged
@thekip thekip deleted the refactor/extractor branch February 14, 2023 08:35
Martin005 pushed a commit that referenced this pull request Feb 14, 2023
@thekip @Martin005
refactor(extractor): don't use intermediate catalogs (#1358)
fc41be8
Martin005 pushed a commit that referenced this pull request Feb 14, 2023
@thekip @Martin005
refactor(extractor): don't use intermediate catalogs (#1358)
1ec27b5
andrii-bodnar pushed a commit that referenced this pull request Feb 20, 2023
@thekip @andrii-bodnar
refactor(extractor): don't use intermediate catalogs (#1358)
6cfa33d
@thekip thekip mentioned this pull request Feb 21, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Martin005 Martin005 Martin005 approved these changes

@andrii-bodnar andrii-bodnar andrii-bodnar approved these changes

Assignees
No one assigned
Labels
⚠️ breaking change v4
Projects
None yet
Milestone
v4
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@thekip @andrii-bodnar @Martin005