Skip to content
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

jest dependency is not added in the package.json #4145

Closed
apsinghdev opened this issue Dec 14, 2024 · 26 comments
Closed

jest dependency is not added in the package.json #4145

apsinghdev opened this issue Dec 14, 2024 · 26 comments

Comments

@apsinghdev
Copy link
Member

apsinghdev commented Dec 14, 2024

@walterbender I guess @Commanderk3 has mistakenly removed the jest dependency from the package.json file. This results in other contributors not having jest in their local development environment to run tests.

Screenshot 2024-12-14 at 11 40 24 PM

Originally posted by @apsinghdev in #4132 (comment)

@apsinghdev
Copy link
Member Author

apsinghdev commented Dec 14, 2024

@Commanderk3 here is what I am getting to my side.

Screenshot 2024-12-14 at 11 52 08 PM

@Commanderk3
Copy link
Contributor

Commanderk3 commented Dec 14, 2024

@apsinghdev Does your package.json has "test": "jest"

image

@Commanderk3
Copy link
Contributor

If you error is like this :
image

then it is because of newly added config and setup file. After removing these files, it works fine.

@apsinghdev
Copy link
Member Author

@apsinghdev Does your package.json has "test": "jest"

image

It does. My question is, how are you installing Jest in the project? Are you installing using npm or by adding a link directly in the root index.html?

@Commanderk3
Copy link
Contributor

npm

@apsinghdev
Copy link
Member Author

npm

hmm. then shouldn't be there jest reference in devDependencies so that when someone execute npm i first time, jest gets installed to their development env?

@Commanderk3
Copy link
Contributor

it should actually but as I said I kept the package.json file just like @omsuneri edited it the last time. After again doing an :
npm install --save-dev jest
This showed up :->
image

Try installing it again, it should work then.

@omsuneri
Copy link
Contributor

I will be adding all the setup guide for test in a readme soon @apsinghdev @Commanderk3 so no one will get confused while setting up the jest

@apsinghdev
Copy link
Member Author

it should actually but as I said I kept the package.json file just like @omsuneri edited it the last time. After again doing an : npm install --save-dev jest This showed up :-> image

Try installing it again, it should work then.

This is what I am telling. This extra step of installing jest should not be there as we have devDependencies for such tasks.

@omsuneri
Copy link
Contributor

@apsinghdev once there is required to install the jest while setting up then I don't think so

@apsinghdev
Copy link
Member Author

apsinghdev commented Dec 14, 2024

@apsinghdev once there is required to install the jest while setting up then I don't think so

I didn't get it. do you mean adding instructions in readme file of installing jest using npm i --save-dev jest?

@omsuneri
Copy link
Contributor

@apsinghdev yess to guide to use testing

@apsinghdev
Copy link
Member Author

@apsinghdev yes to guide to use testing

Adding instructions on how to run tests is important, but this approach defeats the purpose of the package.json file, which contains all the dependencies we need, including the ones we need in development and production. Also, if each contributor installs jest on their own, they'll end up having different versions of it, and this will cause problems.

@omsuneri
Copy link
Contributor

@apsinghdev I ll try to figure out the best solution for this soon till then we should rely on the same what we are doing.

@apsinghdev
Copy link
Member Author

@omsuneri @Commanderk3 Please don't push unfinished work.
Screenshot 2024-12-15 at 12 32 40 AM

Running npm run test creates many files because there are so many extra configurations that we don't need in the beginning.

Screenshot 2024-12-15 at 12 36 37 AM

I guess @walterbender will also agree on the point that we have to implement the test iteratively. We can write unit tests for files without many advanced configs and later on, can add more things when we feel the need of it.

@omsuneri
Copy link
Contributor

@apsinghdev can you please summarise like what need to be changed

@Commanderk3
Copy link
Contributor

@apsinghdev Got it ! The recent test file is actually a collaborative work. So I pushed my part. But I will keep that in mind.

@omsuneri
Copy link
Contributor

Actually I was writing a test file for platform style and that requires these modules to be exported to run that's the reason for config.js

@apsinghdev
Copy link
Member Author

@Commanderk3 @omsuneri Currently, I am fixing this dependency error and removing the configs we don't need. Let's lay a clear plan for it in tomorrow's meeting

@omsuneri
Copy link
Contributor

@apsinghdev yaa that's might be the better way to resolve

@Commanderk3
Copy link
Contributor

@Commanderk3 @omsuneri Currently, I am fixing this dependency error and removing the configs we don't need. Let's lay a clear plan for it in tomorrow's meeting

That sounds great sir. Also pardon me if I messed up real bad. I look forward to meeting you. 👍

@apsinghdev
Copy link
Member Author

@Commanderk3 @omsuneri Currently, I am fixing this dependency error and removing the configs we don't need. Let's lay a clear plan for it in tomorrow's meeting.

That sounds great sir. Also, pardon me if I messed up real bad. I look forward to meeting you. 👍

You don't need to sorry for this. Just be a little cautious before pushing code. Tests are written nicely.

@omsuneri
Copy link
Contributor

@apsinghdev might be because of me too :/

@omsuneri
Copy link
Contributor

@apsinghdev i think you must close it as the issue is solved

@apsinghdev
Copy link
Member Author

@apsinghdev i think you must close it as the issue is solved

Yeah, make sense.

@omsuneri
Copy link
Contributor

Yuppp. 😊

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants