Skip to content

Conversation

@3w36zj6
Copy link
Member

@3w36zj6 3w36zj6 commented Sep 7, 2025

cf. #233

This pull request introduces a metadata configuration system for the website, centralizing site-wide settings into a new JSON file and schema. It refactors how metadata and translation settings are loaded, making the site easier to configure for different languages and deployments.

@3w36zj6 3w36zj6 force-pushed the feature/add-metadata-json branch from 74e3710 to 00f2f99 Compare September 8, 2025 02:53
@3w36zj6 3w36zj6 requested a review from Copilot September 8, 2025 02:58
@3w36zj6 3w36zj6 marked this pull request as ready for review September 8, 2025 02:58

This comment was marked as outdated.

@3w36zj6 3w36zj6 requested a review from Copilot September 8, 2025 03:05

This comment was marked as outdated.

@3w36zj6 3w36zj6 requested a review from Copilot September 8, 2025 03:12
Copy link
Contributor

Copilot AI left a 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 introduces a centralized metadata configuration system for the website, replacing hardcoded values with a JSON-based configuration approach. The changes enable easier configuration for different languages and deployments by consolidating site-wide settings into a single metadata file.

  • Replaces hardcoded metadata values with a JSON configuration file and TypeScript schema
  • Implements dynamic language switching based on metadata configuration
  • Removes version from package.json in favor of metadata-driven versioning

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
website/src/translation/index.tsx Updates translation system to dynamically select language based on metadata configuration
website/src/metadata.ts Refactors to load all metadata from JSON file instead of hardcoded values
website/package.json Removes version field, now managed through metadata.json
website/metadata.schema.json Adds JSON schema for metadata validation
website/metadata.json Introduces centralized configuration file with all site metadata

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@3w36zj6
Copy link
Member Author

3w36zj6 commented Sep 8, 2025

Could you please do a quick review? @YDX-2147483647 @Its-Just-Nans

Copy link
Contributor

@Its-Just-Nans Its-Just-Nans left a comment

Choose a reason for hiding this comment

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

Yes it's good

Another options would be to use an array destructuring, but the order will be important
const [Translation, translation] = (() => {
  switch (language) {
    case "ja-JP":
      return [JaJPTranslation, jaJPTranslation];
    case "en-US":
      return [EnUSTranslation, enUSTranslation];
    default:
      throw new Error(`Unsupported language: ${language}`);
  }
})();

displayTranslationStatus: boolean;
};

const metadata = metadataJson as unknown as Metadata;
Copy link
Contributor

Choose a reason for hiding this comment

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

as unknown is not necessary here.
Additionally, I remove it and find displayTranslationStatus is missing in metadata.json. 😸

"basePath": {
"type": "string",
"description": "The base public path for deployment. This must match the value used in typst-docs.",
"pattern": "^/$|^/[^/]+/$"
Copy link
Contributor

Choose a reason for hiding this comment

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

I think [^/]+ should be replaced with .+. Otherwise, it won't allow /foo/bar/.

throw new Error(`Unsupported language: ${language}`);
}
})();
export { Translation, translation };
Copy link
Contributor

Choose a reason for hiding this comment

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

If properly set up, then this can be simplified to await import(`./locales/${language}.tsx`). Not sure if it's worth it, though.

https://github.com/rollup/plugins/tree/master/packages/dynamic-import-vars

Properly set up:

  1. Enable top-level await for TypeScript.

    Add "target": "es2017" (or newer) to website/tsconfig.json

  2. Enable dynamic import for Vite.

    • Add a new dependency @rollup/plugin-dynamic-import-vars
    • Add dynamicImportVars({ include: "src/translation/", errorWhenNoFilesFound: true }) (or maybe just dynamicImportVars()) to defineConfig(plugins: [...]) in website/vite.config.ts
  3. Mitigate limitations.

Copy link
Contributor

@YDX-2147483647 YDX-2147483647 left a comment

Choose a reason for hiding this comment

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

LGTM!
The comments are for reference only and can be ignored.

@3w36zj6 3w36zj6 merged commit d43f4ab into main Sep 8, 2025
5 checks passed
@3w36zj6 3w36zj6 deleted the feature/add-metadata-json branch September 8, 2025 05:56
YDX-2147483647 added a commit to YDX-2147483647/typst-jp.github.io that referenced this pull request Sep 8, 2025
YDX-2147483647 added a commit to YDX-2147483647/typst-jp.github.io that referenced this pull request Sep 8, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants