curriculum
curriculum copied to clipboard
Introduction to React Testing: Add missing vitest import statements to testing example.
Because
The testing example from the Introduction to React Testing lesson is missing the vitest import statement, leading to describe, it, and expect not being defined when copy-pasting the example code to follow along.
This PR
- Adds the following import statement to App.test.jsx and App.test.js examples:
import { describe, it, expect } from "vitest";
Issue
N/A
Additional Information
N/A
Pull Request Requirements
- [x] I have thoroughly read and understand The Odin Project Contributing Guide
- [x] The title of this PR follows the
location of change: brief description of change
format, e.g.Intro to HTML and CSS lesson: Fix link text
- [x] The
Because
section summarizes the reason for this PR - [x] The
This PR
section has a bullet point list describing the changes in this PR - [ ] If this PR addresses an open issue, it is linked in the
Issue
section - [x] If any lesson files are included in this PR, they have been previewed with the Markdown preview tool to ensure it is formatted correctly
- [x] If any lesson files are included in this PR, they follow the Layout Style Guide
@TheOdinProject/react I'm not sure why I can't assign a React team member to this PR, but could one of you take a look at this PR.
Hm, in Jest this just works without needing to import describe/it/expect. Is that not the case with vitest @01zulfi?
@dylen400mh If you set globals
to true
in vite.config.ts
file, these imports will be global, so you don't need to import these in each file.
// vite.config.ts file
export default defineConfig({
plugins: [react()],
test: {
globals: true,
environment: 'jsdom',
setupFiles: './tests/setup.ts',
},
})
If you follow the setup instructions, the globals will indeed be set to true.
The warning you're seeing comes from ESLint, but when you run npm run test
, the tests should work.
Maybe we should consider some instructions on updating ESLint rules to avoid the linter warnings?
Yeah, looks like we'll need to calm eslint down. I'll take a look soon
@01zulfi was this something we still need to address?
Just tested this myself and found no eslint issues.
Ran npm create vite@latest foo -- --template react
to scaffold an app, then inside that app did all of the lesson instructions and nothing more. Can confirm doing this raises no eslint errors. That includes checking the ESLint VSCode extension both with the release and pre-release version, and without the Use Flat Config
setting checked (since Vite still scaffolds with ESLint v8.57.0 and .eslintrc, due to eslint-plugin-react not yet supporting v9).
Would someone else be able to check this running through the same instructions? If they can confirm the same "no issues" behaviour, then we needn't action this and can close this PR, if it may have just been an "old version of ESLint thing"
Closing at it doesn't seem this is an issue anymore, and I can't say I've seen it mentioned at all recently in the community. A new issue can always be opened if this changes.
Reopening this because my previous testing was not thorough, as discovered when helping a learner with this issue. I did test the release version of the ESLint extension without setting flat config support
in VSCode settings, and this seemed fine but it turns out I didn't actually restart the extension when switching from the pre-release version.
Restarting the extension highlights this still being an issue, because the eslint config does not recognise Vitest globals.
Adding jest: true
to the env
object in the eslint config fixes most of the globals, but won't cover stuff like vi
, since they're not part of Jest. That would require a manual exception in a globals
object (ESLint doesn't have a vitest
env option).
I propose the most appropriate way to resolve this would be to add the explicit imports in the lesson examples, and also add a note box saying we're doing this despite the previous setup tutorial using globals: true
because of the ESLint extension. Importing would also provide the typedefs for better intellisense anyway. Making eslint aware of the globals will just set them as any
.
@dylen400mh I know it's been a while but is this still something you'd like to work on? If so, the following needs to be done:
- Sync your branch with upstream main and resolve any merge conflicts that arise
- Add a note box immediately after the first test example saying something like
<div class="lesson-note" markdown="1"> #### Vitest globals and ESLint Even if you set `globals: true` in `vite.config.js` like in the setup tutorial, ESLint may still yell at you, as it will not recognize these globals without some extra stuff in your ESLint configuration file. The most straightforward resolution would be to explicitly import the globals you'd need. </div>
Closing due to no response from OP. Will open a new issue for this, seeking assignment.