Remove react-router-dom and fix build on windows#22
Remove react-router-dom and fix build on windows#22duskvirkus wants to merge 4 commits intoquietly-turning:srcfrom
Conversation
|
Woah, thanks for this! :) I'm definitely not a React expert either — the ecosystem moves too quickly for me to keep pace with for small projects like this that I only work on sparingly. I'll need a bit to review react-router vs react-router-dom; I admit I'm not familiar offhand. Hopefully I can find time for that this month. Feel free to ping me here if there's been no activity by the end of May! |
Node on Windows was producing filepaths with backslashes, and my html-to-json script was failing to produce valid urls, as pointed out by #22 This commit uses Node's "path" module to normalize this process.
As #22 pointed out, react-router-dom is not necessary to keep in this project. It was intended as a temporary upgrade shim to ease the transition from router v5 to router v6, ...which was a while ago. So, let's remove it. react-router is all that's needed.
Node on Windows was producing filepaths with backslashes, and my html-to-json script was failing to produce valid urls, as pointed out by #22 This commit uses Node's "path" module to normalize this process.
As #22 pointed out, react-router-dom is not necessary to keep in this project. It was intended as a temporary upgrade shim to ease the transition from router v5 to router v6, ...which was a while ago. So, let's remove it. react-router is all that's needed.
|
@duskvirkus Thanks again for taking the time to investigate this issue and submit a PR! I really appreciate it. 💛 I tried a similar-but-slightly-different fix for allowing Windows dev machines to parse the html files into a JSON string: 14d70de I don't have a Windows dev machine, but it Works In Theory™, based on my read of the Node.js docs. 😅 Do you mind testing if you have a moment? |
2 participants
This is a follow up on itgmania/lua-for-itgmania#2
I didn't think to check to see if the link was broken until after successfully building the site. Looks fine. But this is what I ended up doing to make it build correctly on my system.
I first thought the router was broken. Forgive me if there's some reason you want to keep it. I'm not a react expert but saw that react-router-dom was deprecated so tried removing it. Did not solve the build problem but still including it because why not it's deprecated and I went to the trouble of removing it.
Turns out the issue with building on windows had to do with
src/html-to-json.json windows the path names include\\instead of/breaking how the router is finding the pages. So I fixed it just to check and find that the original issue I was having didn't exist on this updated version of the site.Totally understand if you want to reject this pr because changes were not discussed beforehand.