-
Notifications
You must be signed in to change notification settings - Fork 1
fix: import translations lazily #43
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
base: main
Are you sure you want to change the base?
Conversation
✅ Deploy Preview for typst-docs-web ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
af184c8 to
5f2d7a0
Compare
|
I've updated typst-jp. The preview build should now succeed. |
ae251c3 to
55ef4a1
Compare
|
The Netlify build still fails, because I forgot to update the metadata format in |
55ef4a1 to
ecec2fb
Compare
Implements the idea in typst-jp/docs#303 (comment) Blocks typst-community#21
ecec2fb to
ea5fce6
Compare
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.
Pull request overview
This PR implements lazy loading of translation modules to prevent build failures when using en-US translation without optional social metadata (Discord URLs). The ja-JP translation requires both GitHub and Discord social links, while en-US does not. By switching from static imports to dynamic imports, each translation is only loaded when needed, avoiding errors when building for languages that don't require special metadata.
Key Changes:
- Replaced static imports with lazy dynamic imports in the translation module, using top-level await with an IIFE to conditionally load the appropriate translation based on the configured language
- Removed dummy Discord URL from test metadata, demonstrating that en-US builds no longer require it
- Enhanced the
fetch-docs-assets.shscript with a--skipoption to selectively skip downloading specific assets (docs.json, docs-assets, favicon, metadata), enabling more flexible build configurations
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
src/translation/index.tsx |
Converts static translation imports to lazy dynamic imports using top-level await, with detailed comments explaining the rationale for lazy and static import constraints |
src/metadata.ts |
Removes dummy Discord social link from test fallback metadata, validating that en-US translation works without Discord URLs |
scripts/fetch-docs-assets.sh |
Adds --skip option with validation to selectively skip downloading docs.json, docs-assets, favicon, or metadata files; updates usage documentation |
scripts/netlify-build.sh |
Uses the new --skip flags to avoid downloading docs.json and metadata when building ja-JP, since those are fetched separately |
.github/workflows/ci.yaml |
Adds no-config test matrix to verify builds work without optional metadata; uses --skip flags for metadata and favicon in this configuration |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Note
This PR is ready for review, but let's resolve typst-jp/docs#335 first.
At present, it's impossible to use en-US translation without a discord URL, because the ja-JP translation is always imported and it will throw the error below.
This PR fixes it by making the import lazy.
Links
An alternative approach: typst-jp/docs#303 (comment)
Blocks #21