Skip to content

545 bug i18n is a server side singleton#554

Merged
myieye merged 5 commits intodevelopfrom
545-bug-i18n-is-a-server-side-singleton
Feb 9, 2024
Merged

545 bug i18n is a server side singleton#554
myieye merged 5 commits intodevelopfrom
545-bug-i18n-is-a-server-side-singleton

Conversation

@myieye
Copy link
Copy Markdown
Collaborator

@myieye myieye commented Feb 7, 2024

Resolves #545

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Feb 7, 2024 •
edited
Loading

UI unit Tests

1 tests  ±0   1 ✅ ±0   0s ⏱️ ±0s
1 suites ±0   0 💤 ±0 
1 files   ±0   0 ❌ ±0 

Results for commit c3d26dc. ± Comparison against base commit 0115882.

♻️ This comment has been updated with latest results.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Feb 7, 2024 •
edited
Loading

C# Unit Tests

39 tests   39 ✅  5s ⏱️
 7 suites   0 💤
 1 files     0 ❌

Results for commit c3d26dc.

♻️ This comment has been updated with latest results.

hahn-kev
hahn-kev requested changes Feb 8, 2024
View reviewed changes
Copy link
Copy Markdown
Collaborator

@hahn-kev hahn-kev left a comment

Choose a reason for hiding this comment

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

looks good, I left a couple comments. I didn't do a super through test but I did look at it. Let me know if you want me to check more.

Comment thread frontend/src/routes/(unauthenticated)/sandbox/i18n/ssr-only/+page.ts Outdated
Comment thread frontend/src/lib/i18n/index.ts
Comment thread frontend/src/routes/(unauthenticated)/sandbox/i18n/ssr-only/+page.ts
Comment thread frontend/tests/i18n.test.ts Outdated
@myieye
Copy link
Copy Markdown
Collaborator Author

myieye commented Feb 8, 2024

So, I sort of overlooked the fact that I added type safety to all of our translations in my previous changes. As a result, I actually found a bug. So, instead of chaning the type-safety to a loose-autocomplete (which I almost did), I decided that I like the type safety, so I extended our translation API to allow dynamic sub-keys while still type-checking the root of a key.

I also addressed all your great comments, except I'm continuing to ignore the never dissappearing loading idicator on CSR-only pages.

hahn-kev
hahn-kev approved these changes Feb 9, 2024
View reviewed changes
Copy link
Copy Markdown
Collaborator

@hahn-kev hahn-kev left a comment

Choose a reason for hiding this comment

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

Nice work!

@myieye myieye merged commit 02a32cd into develop Feb 9, 2024
@myieye myieye deleted the 545-bug-i18n-is-a-server-side-singleton branch February 9, 2024 08:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@hahn-kev hahn-kev hahn-kev approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Bug: i18n is a server-side singleton

2 participants

@myieye @hahn-kev