refactor!: strictNullChecks for cli lib packages#2412
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
size-limit report 📦
|
260667d to
5be38af
Compare
andrii-bodnar
left a comment
There was a problem hiding this comment.
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 In most of cases this operator used with Also I didn't want to introduce any functional changes, and additional 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. |
Description
This PR enables typescript's
strictNullChecksfor@lingui/cli@lingui/format-po@lingui/loader@lingui/message-utils@lingui/vite-plugin@lingui/extractor-vue@lingui/metro-transformerPackages still not using
strictNullChecks:format-po-gettextthis package doesn't even havestrict: trueat 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 timeThe most importantly that all "lib" packages such as
@lingui/cliand@lingui/confnow 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
strictNullChecksbecausetype
string | null | undefinedis not assignable tostring | undefinedor vice versastring | nullSo it's better to eliminate one or another and write consisently across codebase.
Many teams opted for the
undefinedonly, for example typescript in theirs codebase uses it. The main reason for that is becauseundefinedcould 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
undefinedonly, hence changes.ExtractedMessageTypevsMessageTypeand their correspondingExtractedCatalogTypeandCatalogTypeNow disticntion between them even more "distinct" and change the real code scenario
Extracted*family used for messages produced by extractor, where versions withoutExtractedare corresponding for messages and catalogs from loaded catalogs.What the diffrence? For example just extracted messages would not have a
translationproperty,obsoleteandextra, they are parts of the locally stored catalog.And vice versa,
ExtractedMessageTypealways have theplaceholdersandcommentsfields, 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
Fixes # (issue)
Checklist