Skip to content
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

Use site configs in site #283

Merged
merged 64 commits into from
Aug 5, 2024
Merged

Use site configs in site #283

merged 64 commits into from
Aug 5, 2024

Conversation

magdalenaxm
Copy link
Contributor

@magdalenaxm magdalenaxm commented Jul 4, 2024


PR Checklist

  • Link to the respective task if one exists: COM-988

@magdalenaxm magdalenaxm self-assigned this Jul 4, 2024
@magdalenaxm magdalenaxm changed the base branch from main to next July 4, 2024 13:47
@magdalenaxm magdalenaxm changed the title Draft: Use site configs in site Use site configs in site Jul 5, 2024
site/src/middleware.ts Outdated Show resolved Hide resolved
This was referenced Jul 5, 2024
@thomasdax98 thomasdax98 self-assigned this Jul 8, 2024
Copy link
Collaborator

@johnnyomair johnnyomair left a comment

Choose a reason for hiding this comment

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

As discussed, please change the site configs so that we have one config for each domain with multiple languages.

@magdalenaxm magdalenaxm removed their assignment Jul 10, 2024
@thomasdax98 thomasdax98 marked this pull request as draft July 11, 2024 14:04
@thomasdax98 thomasdax98 force-pushed the use-site-configs-in-site branch 2 times, most recently from fb99d82 to 2f9c151 Compare July 12, 2024 09:19
@thomasdax98 thomasdax98 force-pushed the use-site-configs-in-site branch 2 times, most recently from 303ddf6 to 3891531 Compare July 12, 2024 11:48
johnnyomair
johnnyomair previously approved these changes Jul 31, 2024
johnnyomair
johnnyomair previously approved these changes Jul 31, 2024
fraxachun
fraxachun previously approved these changes Jul 31, 2024
site/src/middleware.ts Outdated Show resolved Hide resolved
@thomasdax98 thomasdax98 dismissed stale reviews from fraxachun and johnnyomair via 2ad4763 August 1, 2024 12:36
site/src/config.ts Outdated Show resolved Hide resolved
site/src/middleware.ts Outdated Show resolved Hide resolved
site/src/middleware.ts Outdated Show resolved Hide resolved
site/src/middleware.ts Outdated Show resolved Hide resolved
admin/src/App.tsx Outdated Show resolved Hide resolved
}

// Used for getting SiteConfig in server-components where params is not available (e.g. sitemap, not-found - see https://github.com/vercel/next.js/discussions/43179)
export async function getSiteConfig() {
Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure middleware is the right place for this function

Copy link
Contributor

@fraxachun fraxachun Aug 2, 2024

Choose a reason for hiding this comment

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

Me neither, but putting it in config.ts is not possible since this file is called from client components and therefore must not contain server libraries. What about an config.server.ts?

Copy link
Member

Choose a reason for hiding this comment

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

I understand.

You could put it into it's own file?

@thomasdax98 thomasdax98 merged commit 05f8aee into main Aug 5, 2024
3 checks passed
@thomasdax98 thomasdax98 deleted the use-site-configs-in-site branch August 5, 2024 07:12
@johnnyomair johnnyomair mentioned this pull request Aug 6, 2024
1 task
johnnyomair pushed a commit that referenced this pull request Aug 13, 2024
We need to change the icon path now that #283 has been merged.
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.

5 participants