hackforla / HomeUniteUs

We're working with community non-profits who have a Host Home or empty bedrooms initiative to develop a workflow management tool to make the process scalable (across all providers), reduce institutional bias, and effectively capture data.
https://homeunite.us/
GNU General Public License v2.0
35 stars 21 forks source link

Fix mocking errors between Node v16 and Node 18+, clean up repository #665

Closed erikguntner closed 1 month ago

erikguntner commented 2 months ago

What changes did you make?

JpadillaCoding commented 1 month ago

Hey Erik I was reviewing the changes but am having a hard time understanding what the issue was since I never experienced it myself. Could you help me by telling me how I can mimic the bug? I also noticed that up until this point I never had a .env file for the front-end. Will this be necessary for this issue integration? I see that VITE_HUU_API_BASE_URL is used here to address the issue. If so let me know what I would need to setup on that file if anything differs from the .env example. Thanks!

erikguntner commented 1 month ago

@JpadillaCoding @Joshua-Douglas thanks for checking this out! Looks like the Node version was the main culprit. I was running Node v20 while MSW doesn't have support for v18+. That would also explain why it would always pass in CI, but not locally since we use v16 in GitHub actions. @JpadillaCoding were you using Node v16 as well?

It is probably worth noting that I ran each test case without a .env file present. It is possible that the .env file may interfere with our tests.

It looks like whether you have the .env file or not doesn't matter since import.meta.env.VITE_HUU_API_BASE_URL gets set in vite.config.ts.

I think these changes combined with the changes @mira-kine proposed in #638 would help make our codebase more compatible with Node versions 18+ going forward.

erikguntner commented 1 month ago

@Joshua-Douglas @JpadillaCoding I made some new updates to this that I think cleaned up some of the issues with the original changes.

Please let me know if you have any further questions about the updates. Thanks!

JpadillaCoding commented 1 month ago

Hey @erikguntner sorry for the late response. I've pulled your changes and performed npm install. I'm using node version 21.7.1 and every test is passing except for ForgotPassCode.test.tsx. Oddly enough when I use v16.0.0 I get 9 file failures when Joshua is having them all pass. When using v20 or v18 I get them all to pass no problem, so the node migration should be helpful for this issue. Everything looks good to me and working as expected with the changes made. Thanks!

erikguntner commented 1 month ago

I'm using node version 21.7.1 and every test is passing except for ForgotPassCode.test.tsx.

Hmm could you provide more info for this? Might be a Node v21 issue?

Oddly enough when I use v16.0.0 I get 9 file failures when Joshua is having them all pass.

This should be the case since MSW v2 doesn't support v16. I think they were passing for Josh since he was still on main and hadn't pulled in the changes yet. @Joshua-Douglas is that right?

When using v20 or v18 I get them all to pass no problem, so the node migration should be helpful for this issue.

That's the ultimate goal. Since v16 is coming to its end of life, these package upgrades will get us off of v16 for the most part and make us compatible with node v18+.