-
Notifications
You must be signed in to change notification settings - Fork 358
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
Tech Story: [M3-8281] - Introduce Mock Service Worker v2 #10610
Conversation
e464335
to
c0ee5b6
Compare
ec00e02
to
c275b5a
Compare
8443cd0
to
150d3ec
Compare
c1ff551
to
60e983e
Compare
@@ -1,6 +1,7 @@ | |||
import { COUNTRY_CODE_TO_CONTINENT_CODE } from './constants'; | |||
|
|||
export type Capabilities = | |||
| 'Backups' |
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.
An interesting omission which @jdamore-linode added initially. Def returned by the /regions
endpoint. Not adding a changeset for it since it's not very impacting.
</div> | ||
<div className="dev-tools__tool__footer"> | ||
<div className="dev-tools__button-list"> | ||
<button onClick={resetFlags}>Reset to LD DEV Defaults</button> |
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.
Mostly things moving around
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.
Agree with switching Grid
s to div
s -- no need for fancy themed components in devtools
</button> | ||
</div> | ||
</div> | ||
</div> |
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.
It's a pretty large file, which could be broken down further (already exported some components, constants, types and methods)
], | ||
regions: [...initialContext.regions, ...(seedContext?.regions || [])], | ||
volumes: [...initialContext.volumes, ...(seedContext?.volumes || [])], | ||
}; |
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.
This could be certainly be improved and that's on my list. In general this file is dense, complex and lacking optimization which I plan to work on and test. Out of scope for this first rendition as this isn't client facing code.
if (container) { | ||
const root = handleRoot(container); | ||
root.render(<Main />); | ||
} |
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.
See handleRoot()
definition. This fix is to circumvent HMR issues while developing Dev Tools. It should be non-impacting but need to point out the change in self review since it touches our APP entry point and needs to be looked at accordingly.
Co-authored-by: Mariah Jacobs <114685994+mjac0bs@users.noreply.github.com>
Co-authored-by: Mariah Jacobs <114685994+mjac0bs@users.noreply.github.com>
Co-authored-by: Mariah Jacobs <114685994+mjac0bs@users.noreply.github.com>
Co-authored-by: Mariah Jacobs <114685994+mjac0bs@users.noreply.github.com>
Co-authored-by: Mariah Jacobs <114685994+mjac0bs@users.noreply.github.com>
d8c5766
to
b0e2e38
Compare
@coliu-akamai it is because we simulate a shut down + corresponding eventing so yeah it takes extra time. may be worth adjusting this later on |
8aab40a
to
b8a7d14
Compare
Description 📝
🔧 This PR implements the first iteration of MSW tooling V2.
🙏 I apologize for throwing 5000+ lines at you all. But hey, we may all benefit from this in the near future. It was already a bit too late to break it down when inherited then it grew some more. Apart for a few files, it is very contained code, and not bundled with our production application. To that effect it can be imperfect (and it is) and is very subjected to iteration and contributions.
👀 Please read the whole PR description before reviewing to manage certain expectations - especially regarding:
As we are reducing our reliance on ALPHA (lower env getting deprecated), development in Cloud Manager needs a better way of emulating what developing against a real API feels like. Our previous MSW tooling, while very helpful, is very static and lacks model relationships among other things (aka, update this volume which should update this linode).
This is a problem when moving forward with a feature we only can assume the behavior of, especially with the reactivity that comes with react-query.
While big and fairly complex, this contribution only builds the foundation for a full suite of tools aimed to facilitate local development for existing and new features. It features current best practices for building upon it by providing both simple and complex examples on how to mock our features.
Warning
This is (and will remain for a while) very much a work in progress! There will be bugs, which is acceptable considering this is a non-public facing development tool. I encourage contributions and engagement for the team who will end up benefiting greatly from fine tuning this suite. However, it needs to be used in order to grow, and for that reason can't be nursed forever, and needs to be merged as an in-progress suite. As such I also encourage contributions as feedback!
What it does:
What is next
Note
While pushing forward a new approach, legacy MSW tooling is available as a baseline preset. We do want to write any new handlers in the new CRUD mode, but we can still use old tooling as "read-only", at least until the new suite is mature and well rounded.
CREDITS for @jdamore-linode for the initial effort and pushing for a brave new world 🤟
Preview 📷
How to test 🧪
Best way is to play with the tool and read along the documentation!
💡 I plan to pick and choose what feedback can be implemented as this point, since the PR is already large. Important bugs will be addressed, feature requests will be captured for subsequent tickets.
Important
Besides new tooling bugs, these are the two items we want to be confident about:
As an Author I have considered 🤔
Check all that apply