-
Notifications
You must be signed in to change notification settings - Fork 60
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
feat(coral): Setup msw to mock api [#144] #172
Conversation
924ffdd
to
a9a4532
Compare
f874a80
to
52337df
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Couple of improvement suggestion of following areas:
- Implement deferred mounting for browser environment
- Define mocked handlers closer where they are used / needed instead in the API domain.
- Move
api-mocks
from domain into services.
Also, we could discuss about if the mock server used in tests should have handlers that catch everything and raise an exception. That would prevent all outgoing API requests and nudge the developer to implement a proper mock. The following handlers could be initialized in the startup:
rest.get('*', throwError),
rest.put('*', throwError),
rest.post('*', throwError),
rest.delete('*', throwError),
efebc8c
to
427c00e
Compare
- `mockServiceWorker.js` is not in `public` to avoid it being build for prod - `main.tsx` required a special handling to make sure msw is not build for prod
- to proof that setup works for app and tests.
- files and directories match architecture plan closer - remove unneeded handlers in mock handles - add typing
- bypass warning for HMR, setting was missing before (https://mswjs.io/docs/api/setup-worker/start#onunhandledrequest) - updated msw directory in package.json
- expose a msw instance in global window to enable usage in components - move mocked api calls to actual api calls - mock API call in component HomePage directly instead of global
- add custom type for window - add types for usage of msw - only run mocked api call in component when msw is set on window (indicating a worker running in browser env) - update test for HomePage
427c00e
to
0a955cd
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Couple of more comments, but I think the big picture is now starting to look good 👍
Note |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Lets extend the global window type with msw. After that I will do a final testing on my end and lets get this in .
eaba02e
to
1be693b
Compare
Note
I created a follow up ticket for this epic to use the mocked API in
Login
instead ofHomePage
. I removed that from the scope of this PR because it already has a lot of changes and I didn't want to make it too big.About this change - What it does
Resolves: #144
Why this way
There where a few tweaks needed to make it work properly with Vite:
mockServiceWorker.js
is not inpublic
since everything in public is in prod buildmain.tsx
follows solutions from this discussionNote: I checked the build files and msw is correctly not introduced there 🎉