-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
refactor(docs): mocking restructure #441
refactor(docs): mocking restructure #441
Conversation
✔️ Deploy Preview for vitest-dev ready! 🔨 Explore the source changes: a045a07 🔍 Inspect the deploy log: https://app.netlify.com/sites/vitest-dev/deploys/61e178282d30ed00089047f7 😎 Browse the preview: https://deploy-preview-441--vitest-dev.netlify.app |
I'm not entirely sure if this makes things better. First I've tried to add a folder with the mocking docs and referencing those as children in the sidebar navigation. This does result in a clean sidebar but breaks the sidebar when the subnavigation gets too deeply nested. I also don't think it's the easiest to maintain. After that I've reworked the mocking section to be in line with the rest of the sections (and the vite docs) but not sure if it makes it easier to navigate. I've also checked the config and api pages and for them to be more like vite docs I think it doesn't improve. To be 100% honest, the vite docs aren't that great to find your way around IMO. There is another concept I've been exploring during this refactor and it does seem like it would improve maintainability as well as readability. But it does seem to break the sidebar navigation. I'm also not sure if anyone would frown upon the approach to be honest 😅 I did the following;
This does work but it does seem to break the sidebar navigation in the way that only the top level headers defined in the parent markdown file are rendered, but not the navigation with the imported markdown files (as Vue components). Long story short, I believe we have some great sections that explain and take the user where they need to go fast that don't adhere to the vite docs practice perse. So I believe we should either choose to go full on vite docs copy paving our own way while not reinventing the wheel by taking the vite docs into consideration. @Snugug would also love to hear your thoughts on the matter. p.s here is a quick link to the "updated" section |
Thanks for checking how this refactoring looks. I agree with you that the result doesn't seem to improve things in the end. I think that each mocking section is better as a separate guide, it is good for example that when you are looking into one of them you don't see the full expanded subtitles for all other mocking guides. I don't think we should add hacks with intermediate vue components. @mitchelvanbever what do you think about having the Mocking guides in a new separate Mocking category like the API category in the vite docs. Check the sidebar here https://vitejs.dev/guide/ |
I think the main problem of mocking guides (except for request) is that it's too much of an |
I think that could work, will update the PR accordingly.
I agree that the mocking section at the moment is not consistent in terms of content. I can have a look at that as well but I don't think it would solve the issue of it being more readable in the current state. Let's discuss how we can divide it in a way that makes more sense once we settle on the structure 🙂 |
I think the separate section does make it easier to navigate. Now I think it's just a matter of curating and moving the content accordingly (as @sheremet-va has pointed out). Do we want to all the mocking files to be more in line with requests or vice versa? |
I've started moving some of the content locally, how do we feel about this? I'll keep the Mocking chapters on the guide page as well but will rewrite those to be a little more like the requests section (with links to the api section for just the actual definitions). Let me know if you all think this would be a good direction to go. If so I'll continue this path later today 🤙 |
Yes, I think this is going to help a lot. Thanks @mitchelvanbever ! |
Hey all! You've covered a bunch of my thoughts already, but I'll just re-state what I think are the key ones here:
|
@Snugug we may get clean URLs soon vuejs/vitepress#477 |
Thanks for the input ya'll ❤️ Just pushed un update that separates the This is a work in progress as I pretty much moved all the content from the examples but I will be working to bring those back. Keeping in mind @Snugug remarks, having them be more use-case focussed as well as giving a more high-level overview of the mocking basics to mitigate common pitfalls. I blatantly copied and pasted everything from the guide section over to the API section, so haven't looked at the content for API page specifically so feel free to drop suggestions if you feel things could be abbreviated (or adjusted in any other way) for the new API sections 🙂 |
I think with the new separation of concerns it isn't as cluttering as the chapters it had previously. So I feel that |
Apart from the module mocking example I believe this is done and ready for review. Will leave it in draft however as we haven't decided if we're going to roll with this structure. |
Hey all! Just gave this a quick preview (what's live and staged, that is). I think the current structure is a big improvement on what's currently live, so I think we should move forward with it as-is and if/when we can update the routing with Vitepress, we can go ahead and do that later. I'll work this weekend to give it a proper tech writing review. Thanks everyone for your awesome work! |
@Snugug do you prefer to review in this PR, or merge and later send modifications in a future PR? |
My usual preference would be to review this PR before merge, but I'll defer to your preference here.
… On Jan 8, 2022, at 11:00 AM, patak ***@***.***> wrote:
@Snugug do you prefer to review in this PR, or merge and later send modifications in a future PR?
I also think that this is a big jump from what we had before
—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications on the go with GitHub Mobile for iOS or Android.
You are receiving this because you were mentioned.
|
No rush at all, better to me to do it here too 👍 |
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.
Ok, thanks for all of this work! I've left individual comments, most with suggestions ready to merge in. Overall, my comments can summed up as follows:
- Use double quotes (
"
) instead of single quotes ('
) in paragraphs - Remove "simply" and "easily" (and the like) when describing stuff
- Use absolute URL paths instead of relative URL paths when linking to other documents (@patak-dev unless you want these to work in GH too?) and make sure they're pointing to the right place as per the URLs in Netlify
- Where possible, make use cases clear (why as a user would I want to do this) as opposed to what technical need someone would have to use it. As an example, the modules section tells me the technical scenario that would I would use this in, but not the why that's a thing I may need (3P dependency that calls a prod DB, something like that?) A good way to start is to try and translate the examples into a written story explaining what's happening; if you can't do that, the example probably is closer to API documentation than a guide, and both probably should be reworked a little to "tell a story", if that make sense (and if it doesn't, LMK and I'll try explaining a different way!).
Thanks again for all the work on this! It's looking great!
docs/guide/mocking.md
Outdated
|
||
Note that restoring mock from `vi.fn()` will set implementation to an empty function that returns `undefined`. Restoring a `vi.fn(impl)` will restore implementation to `impl`. | ||
## Functions | ||
Mock functions (or "spies") observe functions, that are invoked in some other code, allowing you to test its arguments, output or even redeclare its implementation. |
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.
Mocks and spies aren't quite the same; a spy watches and can verify that something has happened (you can spy on console.log
, for instance), whereas a mock is a fake version of something, often with a spy attached. I'd disconnect the concepts here; focus on spying on existing functions first, then calling a spy function, then mocking functions entirely, and update the example accordingly.
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.
Thanks I will make sure the differences between them becomes more clear. I wasn't sure what the most optimal level of handholding would be but what you are saying makes a lot of sense!
|
||
### mockReturnValue | ||
Mock modules observe third-party-libraries, that are invoked in some other code, allowing you to test arguments, output or even redeclare its implementation. |
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.
Can we have more of a usecase-based explanation as to why someone would want to do this?
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.
I was struggling a bit coming up with an example that wouldn't be too framework specific, but would still be very use-cased focused. I didn't want to go the obvious route of axios or fetch (as we do not recommend mocking
those in the next section 😁 ). A router example would be next on my list but would also contain a lot of framework (or even library specific) API in the example.
So I'm open to suggestions here (if anyone has any) 🙂
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.
What about something like mocking a DB library? Or an authorized SDK? Something where mocking just the request isn't sufficient (I think of like Firebase's Firestore which needs a separate emulator and auth to work and the network requests it makes are opaque; you could just mock the module instead to test data in and out)
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.
I've updated the example and went for the db one but I do think this is now the only example that won't run out of the box (a.k.a copy pasted into vite.dev/new) and I'm not sure if this makes things clearer, but it is a more usecase focussed example. Let me know what you think 👌
This one is important. I always need to review my own writing to remove these words, we are so used to them.
What is the rationale for preferring absolute URLs? I think that relative URLs are better so the result can be deployed to other platforms, or to netlify but as preview deploys. Like the one at the top of this PR: https://deploy-preview-441--vitest-dev.netlify.app |
Co-authored-by: Sam Richard <[email protected]>
Sorry, when I say "absolute" I mean `/api` instead of `../api`
… On Jan 9, 2022, at 9:26 AM, patak ***@***.***> wrote:
Remove "simply" and "easily" (and the like) when describing stuff
This one is important. I always need to review my own writing to remove these words, we are so used to them.
Use absolute URL paths instead of relative URL paths when linking to other documents ***@***.*** unless you want these to work in GH too?) and make sure they're pointing to the right place as per the URLs in Netlify
What is the rationale for preferring absolute URLs? I think that relative URLs are better so the result can be deployed to other platforms, or to netlify but as preview deploys. Like the one at the top of this PR: https://deploy-preview-441--vitest-dev.netlify.app
—
Reply to this email directly, view it on GitHub, or unsubscribe.
Triage notifications on the go with GitHub Mobile for iOS or Android.
You are receiving this because you were mentioned.
|
Updated PR and implemented feedback. Let me know what you guys think of the 'new' mocking example |
Fixes #429