Skip to content

Conversation

@Its-Just-Nans
Copy link
Contributor

@Its-Just-Nans Its-Just-Nans commented Aug 14, 2025

This pull request improves how URLs are constructed and managed throughout the website by centralizing the base URL and using it to generate absolute URLs for metadata, social sharing, and links to the official Typst site. This reduces hardcoded strings, making the codebase easier to maintain and update.

@Its-Just-Nans
Copy link
Contributor Author

Note: font in globals.css need to be also edited

@Its-Just-Nans
Copy link
Contributor Author

@3w36zj6 3w36zj6 requested review from 3w36zj6 and Copilot August 14, 2025 04:10

This comment was marked as outdated.

Copy link
Contributor Author

@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.

?

Copy link
Member

@3w36zj6 3w36zj6 left a comment

Choose a reason for hiding this comment

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

Metadata is centrally managed in website/src/metadata.ts. Additionally, please use the URL object for URL concatenation.

// website/src/metadata.ts
export const baseUrl = "https://typst-jp.github.io/"

@Its-Just-Nans
Copy link
Contributor Author

Okay I think it's ready

@3w36zj6
Copy link
Member

3w36zj6 commented Aug 15, 2025

I don't see any major issues overall.
For the record and for other maintainers, let's clarify the responsibility of this PR. Could you please describe in the PR description what problem this PR solves, how it solves that problem, and how the validity of these changes can be manually tested?

@Its-Just-Nans
Copy link
Contributor Author

If you build the website to be relative, for example to host it in a subfolder like

domain.com/mytypsywebsite/

The assets links will be relative to the BASE_URL
So you will set the BASE_URL to "/mytypsywebsite/"

@YDX-2147483647
Copy link
Contributor

YDX-2147483647 commented Aug 15, 2025

The URLs in BaseTemplate.tsx might be able to simplify to just /assets/…, if the base config is set in vite.config.ts or CLI?
Update: No, font URLs are not transformed by vite.

JS-imported asset URLs, CSS url() references, and asset references in your .html files are all automatically adjusted to respect this option during build.

The exception is when you need to dynamically concatenate URLs on the fly.
https://vite.dev/guide/build.html#public-base-path

Note that you should always reference public assets using root absolute path - for example, public/icon.png should be referenced in source code as /icon.png.
https://vite.dev/guide/assets.html#the-public-directory

(I didn't verify it with Hono JSX, however.)


Besides, VitePress has a withBase helper. Its source code is as the following.

export const EXTERNAL_URL_RE = /^(?:[a-z]+:|\/\/)/i

/**
 * Join two paths by resolving the slash collision.
 */
export function joinPath(base: string, path: string) {
  return `${base}${path}`.replace(/\/+/g, '/')
}

/**
 * Append base to internal (non-relative) urls
 */
export function withBase(path: string) {
  return EXTERNAL_URL_RE.test(path) || !path.startsWith('/')
    ? path
    : joinPath(siteDataRef.value.base, path)
}

@Its-Just-Nans
Copy link
Contributor Author

The URLs in BaseTemplate.tsx might be able to simplify to /assets/…, if the base config is set in vite.config.ts or CLI?

BASE_URL is the base in vite.config.ts

https://vite.dev/guide/env-and-mode.html#built-in-constants

@YDX-2147483647
Copy link
Contributor

YDX-2147483647 commented Aug 15, 2025

Excuse me, what is the proper way to build now? I've tried the following, but it doesn't work…
EDIT: I try to edit vite.config.ts instead. Now it works, except index.tsx and SearchWindow.tsx.

mise run generate-docs
cd ./website/

mise exec -- bun run -- vite build --base=/sub/
mise exec -- bun run update-search-index

mise exec -- bun run preview --base=/sub/
# Then go to http://localhost:4173/sub/docs/

Additionally, should index.tsx and SearchWindow.tsx also be updated?

https://github.com/typst-jp/typst-jp.github.io/blob/739d975e8b54c1c65158be1b242ee6809378d411/website/src/index.tsx#L29-L31
https://github.com/typst-jp/typst-jp.github.io/blob/739d975e8b54c1c65158be1b242ee6809378d411/website/src/components/ui/common/SearchWindow.tsx#L7-L8

@3w36zj6
Copy link
Member

3w36zj6 commented Aug 15, 2025

There are probably two ways to achieve this.

One is to change base in docs/src/lib.rs and generate the documentation JSON.
The other is to add the base to the path in the Hono routing definition.

I forgot about Pagefind. We need to handle this as well. However, it might be better to handle the search index in a separate PR. It's preferable to keep each PR small and focused.

@YDX-2147483647
Copy link
Contributor

YDX-2147483647 commented Aug 15, 2025

One is to change base in docs/src/lib.rs and generate the documentation JSON.

The typst-docs crate has added an official CLI in #3429 after typst-jp.github.io started. It is capable of changing base.
However, this crate has been deeply customized in this repo. I'm not sure if there are any other issues.

- cargo test --package typst-docs --lib -- tests::test_docs --exact --nocapture
+ cargo run --package typst-docs -- --assets-dir assets/docs --out-file assets/docs.json --base /sub/docs/

https://github.com/typst-jp/typst-jp.github.io/blob/739d975e8b54c1c65158be1b242ee6809378d411/docs/src/main.rs#L104-L112

@3w36zj6
Copy link
Member

3w36zj6 commented Aug 15, 2025

The typst-docs crate has added an official CLI in #3429 after typst-jp.github.io started. It is capable of changing base.

This looks good. I'll give it a try as well.

@Its-Just-Nans
Copy link
Contributor Author

oh nice cargo commands

Check again this multilang website

I'm updating this MR this it's not useful anymore

@Its-Just-Nans Its-Just-Nans requested a review from 3w36zj6 August 15, 2025 18:00
@Its-Just-Nans Its-Just-Nans changed the title always use relative use metadata.ts Aug 15, 2025
@3w36zj6 3w36zj6 changed the title use metadata.ts refactor: use metadata constants for absolute URLs in website Aug 16, 2025
@3w36zj6 3w36zj6 requested review from Copilot and gomazarashi August 16, 2025 16:44
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 refactors hardcoded absolute URLs to use centralized metadata constants, improving maintainability by reducing duplication and making URL management more consistent across the website.

  • Adds a baseUrl constant to the metadata configuration
  • Replaces hardcoded URLs in Vite configuration with dynamic URL construction
  • Updates React template to use URL construction from metadata constants

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File Description
website/src/metadata.ts Adds baseUrl constant for centralized URL management
website/vite.config.ts Updates sitemap and robots.txt configuration to use metadata constants
website/src/components/templates/BaseTemplate.tsx Replaces hardcoded URLs with dynamically constructed URLs using metadata

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

Copy link
Member

@kimushun1101 kimushun1101 left a comment

Choose a reason for hiding this comment

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

Thank you for your contribution. The following command allowed me to verify the display in my local environment:

git clone https://github.com/Its-Just-Nans/typst-jp.github.io.git temp
cd temp
git switch make-relative
mise trust
mise run generate
mise run dev # also `mise run preview`

@3w36zj6
Copy link
Member

3w36zj6 commented Aug 18, 2025

This is a minor change, so I will merge it.

@3w36zj6 3w36zj6 merged commit a51a135 into typst-jp:main Aug 18, 2025
5 checks passed
@3w36zj6 3w36zj6 linked an issue Aug 19, 2025 that may be closed by this pull request
12 tasks
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