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

Better langugage support (and refactor and update a lot of dependencies) #78

Merged
merged 17 commits into from
Jul 4, 2024

Conversation

Herkarl
Copy link
Member

@Herkarl Herkarl commented Jun 30, 2024

Add support for the changes introduced in taitan#33 and bawang-content#534.

Ie:

  • Language is decided by a get parameter lang instead of by having an /en in front of the path.
  • The language button at the top keeps you at the same page.

Also includes a lot to refactoring and dependency updating since I wanted to use features that only existed in newer versions of libraries, and since that broke stuff I decided to just update and fix everything.

Currently not mergeable since a bug appeared causing all links etc to stop working sometimes, with the following error message: Uncaught Error: Hydration failed because the initial UI does not match what was rendered on the server.

I also want to add an error page for when a page does not exists in a language.

Everything I've tested seems to have worked so far, but I might have missed something.

Fixes #32 and datasektionen/bawang-content#358

Copy link
Member

@foodelevator foodelevator left a comment

Choose a reason for hiding this comment

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

The website became pretty slow (took almost a second after clicking a link) when running start:dev but was fine when doing a build.

I did however not encounter any hydration errors as you did, even after clicking a lot of links. (on node v20.12.2 and firefox 126.0.1)

It also seems like the language switching button always makes a full page reload which shouldn't be needed but I think that's from before and it's probably hard since it is in Methone.

package.json Outdated Show resolved Hide resolved
src/components/App.js Outdated Show resolved Hide resolved
@Herkarl
Copy link
Member Author

Herkarl commented Jul 2, 2024

The website became pretty slow (took almost a second after clicking a link) when running start:dev but was fine when doing a build.

Hmm, that seems to be true...

I did however not encounter any hydration errors as you did, even after clicking a lot of links. (on node v20.12.2 and firefox 126.0.1)

Interesting, maybe worth testing in v22 too, since that is what we're running on prod?

It also seems like the language switching button always makes a full page reload which shouldn't be needed but I think that's from before and it's probably hard since it is in Methone.

Yeah, thought about that too, but downprioritized to get it done.

@foodelevator
Copy link
Member

I think it's good enough with an error page for when the page doesn't exist in the given language. Unless there are a bunch of hydration errors that is. I guess one could just try by running in docker locally, that shouldn't be too hard.

@Herkarl
Copy link
Member Author

Herkarl commented Jul 2, 2024

I think it's good enough with an error page for when the page doesn't exist in the given language. Unless there are a bunch of hydration errors that is. I guess one could just try by running in docker locally, that shouldn't be too hard.

There is actually an error page from before, and it should show the new error now too. But it seems to have broken? (I'm assuming due to some dependency update). I have tried a bit to get it to work, but haven't really found the cause. Too tired to continue with it now though.

But everything is building with docker as well, and seems to be running great. I even don't get the hyrdation error when running it that way. So the only thing that is in the way now is the error page I suppose, which is not critical if we want to get it up asap.

@Herkarl Herkarl marked this pull request as ready for review July 3, 2024 07:57
@Herkarl Herkarl requested a review from foodelevator July 3, 2024 08:01
Copy link
Member

@foodelevator foodelevator left a comment

Choose a reason for hiding this comment

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

There are some quirks like the client-side routing completely shitting itself making going back to the previous page do nothing when you encounter a page that didn't exist in the given language, but that was probably a problem before as well.

I think it's good enough to deploy! (after removing the console.log haha)

}}
</DataLoader>
export const Taitan = ({ pathname, children, lang }) => {
console.log(pathname)
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
console.log(pathname)

pliz remove

Copy link
Member Author

Choose a reason for hiding this comment

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

ooops

possibly the last????
didnt find more with project search
@Herkarl
Copy link
Member Author

Herkarl commented Jul 3, 2024

There are some quirks like the client-side routing completely shitting itself making going back to the previous page do nothing when you encounter a page that didn't exist in the given language, but that was probably a problem before as well.

I think this is related to the hydration issues, (which I have discovered is why the error page is broken). I think I've discovered the exact problem, but haven't found a good solution yet.
So it is probably worth merging before the fix, in order to not risk stalling the reception work.

@Herkarl Herkarl merged commit 5792e6b into master Jul 4, 2024
1 check passed
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.

Implement better system for switching between english and swedish pages
2 participants