Skip to content

refactor!: strictNullChecks for cli lib packages#2412

Merged
andrii-bodnar merged 13 commits intonextfrom
strictNullChecks
Jan 27, 2026
Merged

refactor!: strictNullChecks for cli lib packages#2412
andrii-bodnar merged 13 commits intonextfrom
strictNullChecks

Conversation

@timofei-iatsenko
Copy link
Collaborator

@timofei-iatsenko timofei-iatsenko commented Jan 26, 2026

Description

This PR enables typescript's strictNullChecks for

  • @lingui/cli
  • @lingui/format-po
  • @lingui/loader
  • @lingui/message-utils
  • @lingui/vite-plugin
  • @lingui/extractor-vue
  • @lingui/metro-transformer

Packages still not using strictNullChecks:

  • format-po-gettext this package doesn't even have strict: true at this point, so it requires a heavy modernizing (in a separate PR)
  • babel-plugin-lingui-macro - many patterns in the code hard to rewrite, currently benefits are not worth the effort, maybe re-attempt next time

The most importantly that all "lib" packages such as @lingui/cli and @lingui/conf now fully strict, that means rewriting "leafs" from dependency graph packages is unblocked.

null vs undefined

Many of the changes in the PR caused by null vs undefined change. There is a lot of debates in typescript and javascript community about when and what should be used. This become even more problematic with strictNullChecks because

type string | null | undefined is not assignable to string | undefined or vice versa string | null

So it's better to eliminate one or another and write consisently across codebase.

Many teams opted for the undefined only, for example typescript in theirs codebase uses it. The main reason for that is because undefined could not be complietely eliminated - it would pops up when property or parameter is optional, but null could be fully eliminated from the code base if you are not assigning it manually.

I also opted for undefined only, hence changes.

ExtractedMessageType vs MessageType and their corresponding ExtractedCatalogType and CatalogType

Now disticntion between them even more "distinct" and change the real code scenario Extracted* family used for messages produced by extractor, where versions without Extracted are corresponding for messages and catalogs from loaded catalogs.

What the diffrence? For example just extracted messages would not have a translation property, obsolete and extra, they are parts of the locally stored catalog.

And vice versa, ExtractedMessageType always have the placeholders and comments fields, even if they are empty, but catalog from disk could omit them.

So to further harden the typesafety this distiction was improved.

Breaking changes description

It's not intentionally breaking change, but changing the types might break something for anyone who used lingui internal types (custom integrations)

Types of changes

  • Bugfix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation update
  • Examples update

Fixes # (issue)

Checklist

  • I have read the CONTRIBUTING and CODE_OF_CONDUCT docs
  • I have added tests that prove my fix is effective or that my feature works
  • I have added the necessary documentation (if appropriate)

@vercel
Copy link

vercel bot commented Jan 26, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Review Updated (UTC)
js-lingui Ready Ready Preview Jan 26, 2026 4:38pm

Request Review

@github-actions
Copy link

github-actions bot commented Jan 26, 2026

size-limit report 📦

Path Size
packages/core/dist/index.mjs 1.77 KB (0%)
packages/detect-locale/dist/index.mjs 618 B (0%)
packages/react/dist/index.mjs 1.21 KB (0%)

@timofei-iatsenko timofei-iatsenko changed the title Strict null checks refactor: strictNullChecks for cli lib packages Jan 26, 2026
@timofei-iatsenko timofei-iatsenko changed the title refactor: strictNullChecks for cli lib packages refactor!: strictNullChecks for cli lib packages Jan 26, 2026
Copy link
Contributor

@andrii-bodnar andrii-bodnar left a comment

Choose a reason for hiding this comment

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

I noticed a frequent use of the Non-Null Assertion Operator !. While this could be ok for Unit tests where you are intentionally passing specific data and want to keep the test code concise, in the library code we risk to have runtime TypeError: Cannot read property... by silencing TypeScript

@timofei-iatsenko
Copy link
Collaborator Author

I noticed a frequent use of the Non-Null Assertion Operator !. While this could be ok for Unit tests where you are intentionally passing specific data and want to keep the test code concise, in the library code we risk to have runtime TypeError: Cannot read property... by silencing TypeScript

this is a consequence for extra type safety enabled by the "noUncheckedIndexedAccess": true,.

In most of cases this operator used with catalogs[locale] or catalog[message] kind of expressions. In this cases we should check that it is not null, but we actually already know that because we just created it few lines above. So it's the case where we can assert that it is not null.

Also I didn't want to introduce any functional changes, and additional ifs and conditional checks is a functional change.

Also covering the existing code with highest level of typesafety is not easy, because the code itself should be written in a way typescript could follow and infer the types.

@andrii-bodnar andrii-bodnar merged commit ece0974 into next Jan 27, 2026
9 checks passed
@andrii-bodnar andrii-bodnar deleted the strictNullChecks branch January 27, 2026 10:10
@andrii-bodnar andrii-bodnar added this to the v6 milestone Jan 29, 2026
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.

2 participants