-
-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
i18n(domains): validation and updated logic #9099
Conversation
|
4a058fd
to
890d91b
Compare
...i18n, | ||
...opts | ||
}); | ||
|
||
export const getRelativeLocaleUrlList = (path = "", opts) => _getLocaleRelativeUrlList({ | ||
base, path, trailingSlash, format, ...i18n, ...opts }); | ||
export const getAbsoluteLocaleUrlList = (path = "", opts) => _getLocaleAbsoluteUrlList({ base, path, trailingSlash, format, site, ...i18n, ...opts }); | ||
export const getAbsoluteLocaleUrlList = (path = "", opts) => _getLocaleAbsoluteUrlList({ | ||
base, path, trailingSlash, format, site, isBuild, ...i18n, ...opts |
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.
looks like getAbsoluteLocaleUrlList
has not changed but is now being passed isBuild
, what is this flag used for?
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 addressed your comment. Very silly from me, good catch.
isBuild, what is this flag used for?
From the PR description:
updated the getAsbsoluteLocaleUrl function, which now accepts domains and a isBuild flag. isBuild is required because we can only use the URLs with domains when building the website, otherwise the dev server is broken
Needs a changeset. |
I will add the changeset at the end. The PR is against the |
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.
One small nit, but looks good to me!
* i18n(domains): validation and updated logic (#9099) * feat(i18n): domain with lookup table (#9112) * chore: add changelog, fix types and enable experimental support in node/vercel * rebase and update lock file * chore: fix failing test * Apply suggestions from code review Co-authored-by: Sarah Rainsberger <sarah@rainsberger.ca> Co-authored-by: Matthew Phillips <matthew@skypack.dev> * Update .changeset/tidy-carrots-jump.md Co-authored-by: Sarah Rainsberger <sarah@rainsberger.ca> * wip * chore: rebase, conflicts and tests * update lock file * chore: correct configuration * chore: correct configuration * fix: regressions * chore: fix conflicts and add more tests * chore: add more validation * chore: more tests and add more restrictions * fix changeset * change and revert adapters * add another restriction * lock file update * Update packages/astro/src/@types/astro.ts Co-authored-by: Sarah Rainsberger <sarah@rainsberger.ca> * Update packages/astro/src/@types/astro.ts Co-authored-by: Bjorn Lu <bjornlu.dev@gmail.com> * wat * fix syntax error * fix config example * Fix for #9673 (#9680) * Fix for #9673 * 🦋 add changeset file * Update breezy-plants-smoke.md Co-authored-by: Florian Lefebvre <contact@florian-lefebvre.dev> * ⚡️ simplified normalizeConfigPath --------- Co-authored-by: Florian Lefebvre <contact@florian-lefebvre.dev> * Fix env var replacement for export const prerender (#9807) * feat(alpinejs): allow customizing the Alpine instance (#9751) * feat(alpinejs): allows customzing the Alpine instance * chore: add e2e tests * fix: rename script * Update index.ts * fix: lockfile * [ci] format * chore: use correct lock file * chore: rebase * fix regressions in tests * fix regressions in tests * fix build * add description * fix missing types * chore: fix tests, again :D * eslint * Update packages/astro/src/@types/astro.ts Co-authored-by: Nate Moore <natemoo-re@users.noreply.github.com> * chore: address feedback * chore: fix regressions * chore: refactor naming * Update packages/astro/src/core/app/index.ts Co-authored-by: Bjorn Lu <bjornlu.dev@gmail.com> * chore: address feedback * update lock file * chore: infer routing from options, not strategy * merge from main * merge from main * Experimental support in vercel adapter * Update packages/astro/src/@types/astro.ts Co-authored-by: Sarah Rainsberger <sarah@rainsberger.ca> * Update packages/astro/src/@types/astro.ts Co-authored-by: Sarah Rainsberger <sarah@rainsberger.ca> * Update .changeset/tidy-carrots-jump.md Co-authored-by: Sarah Rainsberger <sarah@rainsberger.ca> * better changesets * Updates both experimental.i18nDomains and i18ndomains for experimental strategy * fix link syntax * consistent tabs/spaces * Update packages/astro/src/@types/astro.ts Co-authored-by: Sarah Rainsberger <sarah@rainsberger.ca> * apply suggestion --------- Co-authored-by: Sarah Rainsberger <sarah@rainsberger.ca> Co-authored-by: Matthew Phillips <matthew@skypack.dev> Co-authored-by: Bjorn Lu <bjornlu.dev@gmail.com> Co-authored-by: Lou Cyx <git@lou.cx> Co-authored-by: Florian Lefebvre <contact@florian-lefebvre.dev> Co-authored-by: Florian Lefebvre <ematipico@users.noreply.github.com> Co-authored-by: Nate Moore <natemoo-re@users.noreply.github.com>
Changes
This PR starts the work around domain support. Here are the changes:
domains
configuration;domains
support capability. I am not aware of any esoteric case, so notifying Astro withSupportKind
should be enoughgetAsbsoluteLocaleUrl
function, which now acceptsdomains
and aisBuild
flag.isBuild
is required because we can only use the URLs with domains when building the website, otherwise the dev server is brokenCloses: PLT-994, PLT-992 and PLT-1206
Testing
I added various cases to test the implemented features
Docs
Added some basic documentation