fix: [CAL-3880] remove i18n for all <Meta> or figure out how to SSR #15367
Conversation
|
@martinsione is attempting to deploy a commit to the cal Team on Vercel. A member of the Team first needs to authorize it. |
|
Thank you for following the naming conventions! 🙏 Feel free to join our discord and post your PR link. |
github-actions
bot
added
i18n
martinsione
changed the title
Graphite Automations"Add community label" took an action on this PR • (06/07/24)1 label was added to this PR based on Keith Williams's automation. "Add consumer team as reviewer" took an action on this PR • (06/07/24)1 reviewer was added to this PR based on Keith Williams's automation. |
|
|
||
| import type { AppImageProps, MeetingImageProps } from "@calcom/lib/OgImages"; | ||
| import { constructAppImage, constructGenericImage, constructMeetingImage } from "@calcom/lib/OgImages"; | ||
| import { APP_NAME, CAL_URL } from "@calcom/lib/constants"; | ||
| import { buildCanonical, getSeoImage, seoConfig } from "@calcom/lib/next-seo.config"; | ||
| import { truncateOnWord } from "@calcom/lib/text"; | ||
|
|
||
| import text from "../../../../apps/web/public/static/locales/en/common.json"; |
There was a problem hiding this comment.
wouldn't this cause the locales to be downloaded twice? one in the bundle and one in the next.js props?
There was a problem hiding this comment.
I added a bit more context on the PR description but tldr yes it would however I think the size is negligible compared to the problems it solves. With RSC the story is a bit different because you can technically stream compute from the server so would allow you to stream translation from the server directly however I think that would be a far bigger effort. What I also suggested in the description is that if there is interest to adopt this pattern, what we can do is on the useLocale hook that is used everywhere add the english translations as default and when the locale of a user is english avoid fetching the translation file again so this would result on a net zero impact for english users and yes it would have a cost for other users but again, I think the benefits outweighs the costs. What you think @zomars?
Eg this is what people see when they search for "cal com sign up"
What does this PR do?
Notes
I tried to keep it as isolated as possible (just affecting the components that have tag) but I think this "pattern" of defaulting to english when there is no content yet is worth adding it straight to the
useLocalehook because right now google is indexing pages with the keys of i18n elements (If there is an agreement to this I'm happy to move this defaultValue over to theuseLocalecomponent. In terms of weight/size I think it's nothing to worry about as it's only 162 KB, for reference, the signup page takes 4.2MB and part of that is downloading the/en/common.jsonso if there is an agreement to makeuseLocaledefault to english we could potentially avoid fetching/en/common.jsonwhen locale = 'en' resulting in a net zero change in terms of bundle for english users (and minimal for other users).Mandatory Tasks (DO NOT REMOVE)
How should this be tested?
/video/no-meeting-found, right now you'll see "no_meeting_found | Cal.com" with this PR you'll see "No Meeting Found | Cal.com". You can try this on any other page that use<HeadSeo />component.