Skip to content

Fail louder on missing translation data#11776

Merged
aduth merged 9 commits intomainfrom
aduth-missing-translations
Jan 21, 2025
Merged

Fail louder on missing translation data#11776
aduth merged 9 commits intomainfrom
aduth-missing-translations

Conversation

@aduth
Copy link
Contributor

@aduth aduth commented Jan 21, 2025

🛠 Summary of changes

Builds upon #11775 to increase visibility of missing translation strings at build-time and in JavaScript-enabled feature tests:

  • At build-time, fails build if attempting to compile locale data where string data is missing. Previously, this only failed in test environments.
  • At runtime, logs output to console if attempting to reference missing translation string. Previously, this would silently fail and return the original key, resulting in scenarios like what led to Fix string extraction for Webpack function renaming #11775.

The latter works well in combination with feature test behavior to fail tests where any console logging occurs. Unfortunately most tests which involve document capture disable this behavior (allow_browser_log: true), but there's at least one which would have caught the original issue (source).

📜 Testing Plan

Verify that build passes.

Verify that build fails on a commit between 41f8d41 and 73795c9:

git checkout 73795c9
rspec spec/features/idv/steps/in_person/verify_info_spec.rb:122

Comment on lines +31 to +32
if (message) {
const index = console.unverifiedCalls.findIndex((calledMessage) =>
const index = (console as ExtendedConsole).unverifiedCalls.findIndex((calledMessage) =>
Copy link
Contributor

Choose a reason for hiding this comment

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

looks like we have to cast console like 3 times, would it make sense to cast it once?

const logger = console as ExtendedConsole;

Copy link
Contributor Author

Choose a reason for hiding this comment

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

looks like we have to cast console like 3 times, would it make sense to cast it once?

Yeah, I was trying to avoid confusion by naming console anything other than console, but you're right that it turned out rather repetitive. I went with your suggestion in 4f15ecb, but then tinkered with it a bit more and landed at 0735056, avoiding assigning to console altogether.

@aduth aduth merged commit 97a59c0 into main Jan 21, 2025
@aduth aduth deleted the aduth-missing-translations branch January 21, 2025 21:31
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.

3 participants