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.

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

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

Merged
merged 5 commits into from
Oct 27, 2021

Conversation

mszylkowski
Copy link
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

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

@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