-
Notifications
You must be signed in to change notification settings - Fork 504
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
fix(ssr): use different language tag when with duplicate langs #7921
Conversation
Marked as draft to fix mdn/content#23596 (comment) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, just one nit.
ssr/render.ts
Outdated
const LANGUAGE_TAGS = new Map([ | ||
["zh-CN", "zh-Hans"], | ||
["zh-TW", "zh-Hant"], | ||
]); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we use a simple frozen object instead, and add the region?
const LANGUAGE_TAGS = new Map([ | |
["zh-CN", "zh-Hans"], | |
["zh-TW", "zh-Hant"], | |
]); | |
const LANGUAGE_TAGS = Object.freeze({ | |
'zh-CN': 'zh-Hans-CN', | |
'zh-TW': 'zh-Hant-TW', | |
}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure should we just use zh-Hans
for Simplified Chinese (zh-CN) and zh-Hant
for traditional Chinese (zh-TW) without the region code (CN
and TW
). As we are using the shortest language tags that won't be duplicated.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You're right, using zh-Hans
and zh-Hant
would be consistent with using pt
for pt-BR
.
Co-authored-by: Claas Augner <495429+caugner@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tested locally (yarn dev
and looked at the source of http://localhost:5042/en-US/docs/Learn/HTML), and works.
Summary
See the result of hreflang tag testing tool on https://developer.mozilla.org/zh-CN/docs/Web/JavaScript/Reference/Global_Objects/Array:
Problem
lang-region
is not prefered now, we may switch tolang-script
format now. (as a part of <html> lang attribute "zh-CN", "zh-TW" does not conform BCP 47 Language Subtag Registry content#23596)Solution
To resolve this, we should also include zh-CN locale when try to resolve language tag for zh-TW.
Screenshots
Before
See problem description.
After
See testing.
How did you test this change?
run
yarn test:testing