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.

Sign up for GitHub

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

Updated the locale resolver to ECMA402's ResolveLocale #19460

Closed
wants to merge 1 commit into from
Closed

Updated the locale resolver to ECMA402's ResolveLocale #19460

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 34 additions & 4 deletions packages/next/next-server/server/next-server.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,14 +78,14 @@ import { removePathTrailingSlash } from '../../client/normalize-trailing-slash'
import getRouteFromAssetPath from '../lib/router/utils/get-route-from-asset-path'
import { FontManifest } from './font-utils'
import { denormalizePagePath } from './denormalize-page-path'
import accept from '@hapi/accept'
import { normalizeLocalePath } from '../lib/i18n/normalize-locale-path'
import { detectLocaleCookie } from '../lib/i18n/detect-locale-cookie'
import * as Log from '../../build/output/log'
import { imageOptimizer } from './image-optimizer'
import { detectDomainLocale } from '../lib/i18n/detect-domain-locale'
import cookie from 'next/dist/compiled/cookie'
import escapeStringRegexp from 'next/dist/compiled/escape-string-regexp'
import { ResolveLocale } from '@formatjs/ecma402-abstract'

const getCustomRouteMatcher = pathMatch(true)

Expand Down Expand Up @@ -324,10 +324,40 @@ export default class Server {

let defaultLocale = i18n.defaultLocale
let detectedLocale = detectLocaleCookie(req, i18n.locales)

// Parse the `accept-language` header
const regex = /((([a-zA-Z]+(-[a-zA-Z0-9]+){0,2})|\*)(;q=[0-1](\.[0-9]+)?)?)*/g
Copy link
Contributor

Choose a reason for hiding this comment

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

More tests on this would be nice. Better yet if there's already something OSS that gives you the full list we should just use that.

The regex seems to be a little too restrictive compared to the header spec expectation.

Copy link
Member

Choose a reason for hiding this comment

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

Agreed with @longlho

Copy link
Author

Choose a reason for hiding this comment

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

@timneutkens There are already several tests in i18n-support that test the detection from the accept-language header and they all seem to pass. How would you like me to test the regex? Is this something that should be included in the NextJS tests or would you want me to test it on my own (which is not ideal)?

Copy link
Contributor

Choose a reason for hiding this comment

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

At 1st glance seems like regex doesn't fully match es-419-nu-hebr-x-private;q=1 for example

Copy link
Author

Choose a reason for hiding this comment

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

I created the regex based on: https://www.w3.org/Protocols/rfc2616/rfc2616-sec14.html. I may have made a mistake somewhere, but for the moment, I never found an example where the regex was handling the header incorrectly. I think the best way to make sure it works is by trying to find example where it doesn't work (and if we can't then it's fine).

Copy link
Contributor

@longlho longlho Nov 25, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

hmm you prob would have to implement the full RFC4647 (sans the lookup section). RFC2616 has been deprecated since 2014.

Are the no OSS lib for this?

Copy link
Author

Choose a reason for hiding this comment

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

@longlho That may be me not understanding the specification correctly, but to me es-419-nu-hebr-x-private isn't a language-range? According to the specification language-range = ( ( 1*8ALPHA *( "-" 1*8ALPHA ) ) | "*" )

Is that correct or am I wrong?

Copy link
Contributor

@longlho longlho Nov 25, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

The language tag grammar in the rfc you linked has been deprecated since 2014.

Copy link
Author

Choose a reason for hiding this comment

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

Ok, so that's why it got your example wrong. What is the updated specification ?


function parse(al: string = '') {
const strings = al.match(regex) ?? []
return strings
.map((m) => {
if (!m) {
return null
}

const bits = m.split(';')
const ietf = bits[0]

return {
code: ietf,
quality: bits[1] ? parseFloat(bits[1].split('=')[1]) : 1.0,
}
})
.filter((r: any) => r)
.sort((a, b) => b!.quality - a!.quality)
}

const resolvedLocale = ResolveLocale(
new Set(i18n.locales),
parse(req.headers['accept-language']).map((l) => l!.code),
{ localeMatcher: 'best fit' },
[],
{},
() => i18n.defaultLocale
)
let acceptPreferredLocale =
i18n.localeDetection !== false
? accept.language(req.headers['accept-language'], i18n.locales)
: detectedLocale
i18n.localeDetection !== false ? resolvedLocale.locale : detectedLocale

const { host } = req?.headers || {}
// remove port from host and remove port if present
Expand Down
1 change: 1 addition & 0 deletions packages/next/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@
"dependencies": {
"@ampproject/toolbox-optimizer": "2.7.0-alpha.1",
"@babel/runtime": "7.12.5",
"@formatjs/ecma402-abstract": "1.5.0",
"@hapi/accept": "5.0.1",
"@next/env": "10.0.3-canary.2",
"@next/polyfill-module": "10.0.3-canary.2",
Expand Down
52 changes: 51 additions & 1 deletion yarn.lock
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1316,6 +1316,13 @@
resolved "https://registry.yarnpkg.com/@firebase/webchannel-wrapper/-/webchannel-wrapper-0.2.41.tgz#4e470c25a99fa0b1f629f1c5ef180a318d399fd0"
integrity sha512-XcdMT5PSZHiuf7LJIhzKIe+RyYa25S3LHRRvLnZc6iFjwXkrSDJ8J/HWO6VT8d2ZTbawp3VcLEjRF/VN8glCrA==

"@formatjs/[email protected]":
version "1.5.0"
resolved "https://registry.yarnpkg.com/@formatjs/ecma402-abstract/-/ecma402-abstract-1.5.0.tgz#759c8f11ff45e96f8fb58741e7fbdb41096d5ddd"
integrity sha512-wXv36yo+mfWllweN0Fq7sUs7PUiNopn7I0JpLTe3hGu6ZMR4CV7LqK1llhB18pndwpKoafQKb1et2DCJAOW20Q==
dependencies:
tslib "^2.0.1"

"@fullhuman/[email protected]":
version "1.3.0"
resolved "https://registry.yarnpkg.com/@fullhuman/postcss-purgecss/-/postcss-purgecss-1.3.0.tgz#d632900d818f4fcf4678e7326923fb838c3e03a7"
Expand Down Expand Up @@ -4122,7 +4129,25 @@ browserify-zlib@^0.2.0:
dependencies:
pako "~1.0.5"

[email protected], browserslist@^1.3.6, browserslist@^1.5.2, browserslist@^1.7.6, browserslist@^4.0.0, browserslist@^4.12.0, browserslist@^4.14.7, browserslist@^4.3.6, browserslist@^4.6.4, browserslist@^4.8.3, browserslist@^4.8.5:
[email protected]:
version "4.14.6"
resolved "https://registry.yarnpkg.com/browserslist/-/browserslist-4.14.6.tgz#97702a9c212e0c6b6afefad913d3a1538e348457"
integrity sha512-zeFYcUo85ENhc/zxHbiIp0LGzzTrE2Pv2JhxvS7kpUb9Q9D38kUX6Bie7pGutJ/5iF5rOxE7CepAuWD56xJ33A==
dependencies:
caniuse-lite "^1.0.30001154"
electron-to-chromium "^1.3.585"
escalade "^3.1.1"
node-releases "^1.1.65"

browserslist@^1.3.6, browserslist@^1.5.2, browserslist@^1.7.6:
version "1.7.7"
resolved "https://registry.yarnpkg.com/browserslist/-/browserslist-1.7.7.tgz#0bd76704258be829b2398bb50e4b62d1a166b0b9"
integrity sha1-C9dnBCWL6CmyOYu1Dkti0aFmsLk=
dependencies:
caniuse-db "^1.0.30000639"
electron-to-chromium "^1.2.7"

browserslist@^4.0.0, browserslist@^4.12.0, browserslist@^4.14.7, browserslist@^4.3.6, browserslist@^4.6.4, browserslist@^4.8.3, browserslist@^4.8.5:
version "4.14.7"
resolved "https://registry.yarnpkg.com/browserslist/-/browserslist-4.14.7.tgz#c071c1b3622c1c2e790799a37bb09473a4351cb6"
integrity sha512-BSVRLCeG3Xt/j/1cCGj1019Wbty0H+Yvu2AOuZSuoaUWn3RatbL33Cxk+Q4jRMRAbOm0p7SLravLjpnT6s0vzQ==
Expand Down Expand Up @@ -4444,11 +4469,21 @@ caniuse-db@^1.0.30000529, caniuse-db@^1.0.30000634:
version "1.0.30001023"
resolved "https://registry.yarnpkg.com/caniuse-db/-/caniuse-db-1.0.30001023.tgz#f856f71af16a5a44e81f1fcefc1673912a43da72"

caniuse-db@^1.0.30000639:
version "1.0.30001159"
resolved "https://registry.yarnpkg.com/caniuse-db/-/caniuse-db-1.0.30001159.tgz#1983075ffa39c1d96b9e078731215f1fed850bb4"
integrity sha512-fh0J2qYeW36hS5RUT7FnHcvgUxATY5LtzSXURWPuLNAolWaXR+qSu+asX5xcFvC4fwQKzfHWtWuPGJc0VJYrfw==

caniuse-lite@^1.0.0, caniuse-lite@^1.0.30000981, caniuse-lite@^1.0.30001020, caniuse-lite@^1.0.30001093, caniuse-lite@^1.0.30001113, caniuse-lite@^1.0.30001157:
version "1.0.30001157"
resolved "https://registry.npmjs.org/caniuse-lite/-/caniuse-lite-1.0.30001157.tgz"
integrity sha512-gOerH9Wz2IRZ2ZPdMfBvyOi3cjaz4O4dgNwPGzx8EhqAs4+2IL/O+fJsbt+znSigujoZG8bVcIAUM/I/E5K3MA==

caniuse-lite@^1.0.30001154:
version "1.0.30001159"
resolved "https://registry.yarnpkg.com/caniuse-lite/-/caniuse-lite-1.0.30001159.tgz#bebde28f893fa9594dadcaa7d6b8e2aa0299df20"
integrity sha512-w9Ph56jOsS8RL20K9cLND3u/+5WASWdhC/PPrf+V3/HsM3uHOavWOR1Xzakbv4Puo/srmPHudkmCRWM7Aq+/UA==

[email protected]:
version "1.0.0"
resolved "https://registry.yarnpkg.com/capitalize/-/capitalize-1.0.0.tgz#dc802c580aee101929020d2ca14b4ca8a0ae44be"
Expand Down Expand Up @@ -6260,6 +6295,11 @@ ejs@^2.6.1:
version "2.7.4"
resolved "https://registry.yarnpkg.com/ejs/-/ejs-2.7.4.tgz#48661287573dcc53e366c7a1ae52c3a120eec9ba"

electron-to-chromium@^1.2.7, electron-to-chromium@^1.3.585:
version "1.3.604"
resolved "https://registry.yarnpkg.com/electron-to-chromium/-/electron-to-chromium-1.3.604.tgz#285da30e38a71e2b9d28ce3a792ec60235c63b7c"
integrity sha512-Mk5ODhvz+ZaQpVFXbu51wGW94P3CnkJIDkEQGxXMl6Ix6R0PG4IFWz83WbqFEZjN1UksoTsmmzKY5SmUrEvNJQ==

electron-to-chromium@^1.3.591:
version "1.3.596"
resolved "https://registry.yarnpkg.com/electron-to-chromium/-/electron-to-chromium-1.3.596.tgz#c7ed98512c7ff36ddcbfed9e54e6355335c35257"
Expand Down Expand Up @@ -11156,6 +11196,11 @@ node-notifier@^5.4.2:
shellwords "^0.1.1"
which "^1.3.0"

node-releases@^1.1.65:
version "1.1.67"
resolved "https://registry.yarnpkg.com/node-releases/-/node-releases-1.1.67.tgz#28ebfcccd0baa6aad8e8d4d8fe4cbc49ae239c12"
integrity sha512-V5QF9noGFl3EymEwUYzO+3NTDpGfQB4ve6Qfnzf3UNydMhjQRVPR1DZTuvWiLzaFJYw2fmDwAfnRNEVb64hSIg==

node-releases@^1.1.66:
version "1.1.66"
resolved "https://registry.yarnpkg.com/node-releases/-/node-releases-1.1.66.tgz#609bd0dc069381015cd982300bae51ab4f1b1814"
Expand Down Expand Up @@ -15952,6 +15997,11 @@ tslib@^1.8.1, tslib@^1.9.0, tslib@^1.9.3:
version "1.10.0"
resolved "https://registry.yarnpkg.com/tslib/-/tslib-1.10.0.tgz#c3c19f95973fb0a62973fb09d90d961ee43e5c8a"

tslib@^2.0.1:
version "2.0.3"
resolved "https://registry.yarnpkg.com/tslib/-/tslib-2.0.3.tgz#8e0741ac45fc0c226e58a17bfc3e64b9bc6ca61c"
integrity sha512-uZtkfKblCEQtZKBF6EBXVZeQNl82yqtDQdv+eck8u7tdPxjLu2/lp5/uPW+um2tpuxINHWy3GhiccY7QgEaVHQ==

tsutils@^3.17.1:
version "3.17.1"
resolved "https://registry.yarnpkg.com/tsutils/-/tsutils-3.17.1.tgz#ed719917f11ca0dee586272b2ac49e015a2dd759"
Expand Down