HomeUniteUs
HomeUniteUs copied to clipboard
Fix mocking errors between Node v16 and Node 18+, clean up repository
What changes did you make?
- Upgrade MSW to version 2 and refactor mock handlers
- Refactor tests that use mock handlers
- Update the base URL for API calls to use the base URL from the environment
- upgrade actions and Node versions in CI since MSW v2 requires Node 18+
Rationale behind the changes?
- The integration and unit tests using MSW were failing on local machines since the base URL wasn't recognizable in test environments.
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!
@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.
@Joshua-Douglas @JpadillaCoding I made some new updates to this that I think cleaned up some of the issues with the original changes.
- I removed any usage of
import.meta.env.VITE_HUU_API_BASE_URL
that I had previously implemented and changed the API baseUrl to benew URL('/api', location.origin).href
. You can see the changes in commit df48a93. These changes were made as suggested in this issue and the MSW docs. They go into more detail about the issue with relative paths and base URLs in testing environments. - Upgraded Vite, Vitest, @vitejs/plugin-react to the latest versions.
- Removed Storybook. I ran into dependency conflicts with Storybook packages while upgrading Vite, and since Storybook isn't being used, I decided to remove it from the project instead of upgrading the related packages.
- Removed the
/data
and/model
as they were no longer being used either. - Removed the
whatwg-fetch
package. Originally this package poly-filled fetch in the testing environment, but since Node 18+, fetch is globally available so it is no longer needed.
Please let me know if you have any further questions about the updates. Thanks!
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!
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+.