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

Use vitest #260

Closed
wants to merge 0 commits into from
Closed

Use vitest #260

wants to merge 0 commits into from

Conversation

wonkyDD
Copy link
Contributor

@wonkyDD wonkyDD commented Nov 6, 2023

No description provided.

@wonkyDD wonkyDD requested a review from Jonghakseo as a code owner November 6, 2023 10:41
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think good idea is disable prettier for this file, cause this bot add new contributor to this file automattically and don't reformat like prettier.

README.md Outdated
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMO the previouse version is better, cause have short and clear links, paste entire message as a link, isn't good idea.

Create here only the things related to your feature :)

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i think it's bug with prettier, but i think why you run prettier for entire project, i was configure lint-staged, for run prettier and eslint only for staged file.

package.json Outdated
"@types/node": "20.8.10",
"@types/react": "18.2.35",
"@types/react-dom": "18.2.14",
"@types/ws": "8.5.8",
"@typescript-eslint/eslint-plugin": "6.9.1",
"@typescript-eslint/parser": "6.9.1",
"@vitejs/plugin-react": "2.2.0",
"@vitejs/plugin-react": "4.1.1",
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can't update this read more in #258 (comment)

package.json Outdated
"ts-loader": "9.5.0",
"tslib": "2.6.2",
"typescript": "5.2.2",
"vite": "3.2.7",
"vite": "4.5.0",
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can't update this read more in #258 (comment)

package.json Outdated
"ts-loader": "9.5.0",
"tslib": "2.6.2",
"typescript": "5.2.2",
"vite": "3.2.7",
"vite": "4.5.0",
"vitest": "^0.34.6",
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the good approach is don't use '^' anyway, but here we have deps bot, and they are looking for new versions, as you can see in other packages we don't have it, but here @Jonghakseo could say more if he want this, let's do it :)

package.json Outdated
@@ -56,18 +55,17 @@
"eslint-plugin-react-hooks": "4.6.0",
"fs-extra": "11.1.1",
"husky": "8.0.3",
"jest": "29.7.0",
"jest-environment-jsdom": "29.7.0",
"jsdom": "^22.1.0",
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the good approach is don't use '^' anyway, but here we have deps bot, and they are looking for new versions, as you can see in other packages we don't have it, but here @Jonghakseo could say more if he want this, let's do it :)

@wonkyDD wonkyDD closed this Nov 6, 2023
@PatrykKuniczak
Copy link
Collaborator

PatrykKuniczak commented Nov 6, 2023

@wonkyDD Don't panic 😁

Let's open new one or reopen that, if you working on that, you can convert it to draft

@wonkyDD
Copy link
Contributor Author

wonkyDD commented Nov 7, 2023

@PatrykKuniczak
I should have rebased the upstream commit beforehand and fixed the current branch, but I got screwed.
Thanks for the feedback.

You can't update this read more in #258 (comment)

To use vitest successfully, we need to upgrade @vitejs/plugin-react and vite.
If not, it will encounter this issue vitest-dev/vitest#3283 and tests will continue to fail.

However, after reading the issue you linked to, I realized that I am not able to update it at this time.
I should have read the description of the package update PR beforehand, but I didn't. (apologize for bothering you 🙇)

As you said, I think we should also hold off on using vitest until the issue is resolved or
there is a workaround.

Thanks a lot.

@PatrykKuniczak
Copy link
Collaborator

PatrykKuniczak commented Nov 7, 2023

@wonkyDD You don't disturb me 😁
If you have small experience with git don't push force, if you no confident your changes, cause you can destroy you work, create also backup branch before rebase, to hqve possibility to recover your work 👻

I was thinking about workaround, i can equal the source of the file, but there's performance on larger files could be sick.

If i have more time on this month, i can try to solve the PR on rollup, then they can relase new version, and then we can pull this and update, as you can see, everything is blocked 😆

@wonkyDD wonkyDD deleted the use-vitest branch November 20, 2023 01:19
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 this pull request may close these issues.

2 participants