Skip to content

Refactor: Single source of truth for site URL#143

Merged
satnaing merged 4 commits intosatnaing:mainfrom
tanishqmanuja:refactor/website-url
Oct 9, 2023
Merged

Refactor: Single source of truth for site URL#143
satnaing merged 4 commits intosatnaing:mainfrom
tanishqmanuja:refactor/website-url

Conversation

@tanishqmanuja
Copy link
Contributor

@tanishqmanuja tanishqmanuja commented Oct 5, 2023

Note:

import { SITE } from "./src/config"; // ✔️ will work

import { SITE } from "@/config" // ❌ will not work (outside src dir) bc of tsconfig baseUrl;

related: #142

@satnaing
Copy link
Owner

satnaing commented Oct 5, 2023

Which problem can occur when we use or don't use .mjs? What could possibly wrong?
Currently, I see a type error in remarkToc when we use .ts.
However, when I change back to .mjs, that error just gone.

@tanishqmanuja
Copy link
Contributor Author

tanishqmanuja commented Oct 5, 2023

Since the config file has a .ts extension it's not semantically correct to import a uncompiled .ts file into .mjs file. Btw no error for me on remarkToc for both .mjs or .ts

@mattppal
Copy link

mattppal commented Oct 5, 2023

This works for me too— thanks for the update @tanishqmanuja

@satnaing
Copy link
Owner

satnaing commented Oct 6, 2023

In order to resolve the type error, I had to update the dependencies to the latest one.

Copy link
Owner

@satnaing satnaing left a comment

Choose a reason for hiding this comment

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

Just make some minor updates on my own.

astro.config.ts Outdated
// https://astro.build/config
export default defineConfig({
site: "https://astro-paper.pages.dev/", // replace this with your deployed domain
site: SITE.website, // replace this with your deployed domain
Copy link
Owner

Choose a reason for hiding this comment

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

Can you remove (or update) the comment here?
I think this comment is no longer relevant here. Maybe it can be inside the src/config.

@satnaing satnaing merged commit 9dcc5e1 into satnaing:main Oct 9, 2023
@tanishqmanuja
Copy link
Contributor Author

@satnaing, can you add hacktoberfest tag to the repo till nov 1, would be great 😸

@satnaing
Copy link
Owner

@tanishqmanuja
Yep, I wanted to participate in Hacktoberfest as a maintainer. However, I'm a bit hesitant in case there are a lot of pull requests that I have to review.
Besides, at the moment, this repo doesn't have issue templates, review templates, etc.

On the other hand, I'm also thinking of participating in Hacktoberfest.

@tanishqmanuja
Copy link
Contributor Author

I personally think there wouldn't be lots of requests. Plus there is no negative outcome in the case there are, We could solve them one by one as usual :)

@tanishqmanuja tanishqmanuja deleted the refactor/website-url branch April 20, 2025 10:47
qwaxgo pushed a commit to qwaxgo/qwaxgo-astro-template that referenced this pull request Apr 29, 2025
* wip: astro config to use .ts instead of .mjs

* fix: remove invalid prop in astro config

* refactor: single source of truth for site url

* refactor: make minor updates due to astro.config.ts

---------

Co-authored-by: satnaing <satnaingdev@gmail.com>
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.

3 participants