Skip to content

🚀 [Story performance] Remove default translation and use english as default#36632

Merged
mszylkowski merged 5 commits intoampproject:mainfrom
mszylkowski:localization_removedefault
Oct 27, 2021
Merged

🚀 [Story performance] Remove default translation and use english as default#36632
mszylkowski merged 5 commits intoampproject:mainfrom
mszylkowski:localization_removedefault

Conversation

@mszylkowski
Copy link
Copy Markdown
Contributor

@mszylkowski mszylkowski commented Oct 27, 2021

Currently the default translation JSON is separate from en.js but contains the same translations (since default.json is in english). However, default.json is missing many of the translation strings, making it a bad fallback from other languages.

We can remove deafult.json from the codebase and binary, saving 1kb on uncompressed bundle size for amp-story.js while simplifying the selection of fallback localizations and preventing redundancy.

Also, when no language is present, we can just use default (mapped to en.js) instead of ["en", "default"] since no locale bundles have a different default than the english bundle.

amp-story-auto-ads already sets default to en.js which has worked great.

@mszylkowski mszylkowski self-assigned this Oct 27, 2021
@amp-owners-bot
Copy link
Copy Markdown

amp-owners-bot bot commented Oct 27, 2021

Hey @gmajoulet, @newmuis! These files were changed:

extensions/amp-story/1.0/_locales/default.json
extensions/amp-story/1.0/_locales/index.js
extensions/amp-story/1.0/amp-story.js

…mphtml; branch 'main' of github.com:ampproject/amphtml into localization_removedefault
@mszylkowski mszylkowski requested review from dvoytenko and removed request for dvoytenko October 27, 2021 19:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants