Skip to content

Migrate to Vite + Vitest #774

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

Merged
merged 10 commits into from
Sep 26, 2024
Merged

Migrate to Vite + Vitest #774

merged 10 commits into from
Sep 26, 2024

Conversation

MTRNord
Copy link
Contributor

@MTRNord MTRNord commented Sep 6, 2024

✔️ Checklist

  • A changeset describing the change and affected packages (more info).
  • Added or updated documentation.
  • Tests for new functionality and regression tests for bug fixes.
  • Screenshots or videos attached (for UI changes).
  • All your commits have a Signed-off-by line in the message (more info).

Copy link

changeset-bot bot commented Sep 6, 2024

🦋 Changeset detected

Latest commit: d130fa0

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 4 packages
Name Type
@matrix-widget-toolkit/testing Major
@matrix-widget-toolkit/react Patch
@matrix-widget-toolkit/api Patch
@matrix-widget-toolkit/mui Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@MTRNord MTRNord force-pushed the nic/feat/NEO-681 branch 3 times, most recently from 6870309 to b8f226e Compare September 6, 2024 18:28
@MTRNord
Copy link
Contributor Author

MTRNord commented Sep 6, 2024

fb94ea6 exists due to dequelabs/axe-core#4458

@MTRNord

This comment was marked as resolved.

@MTRNord MTRNord marked this pull request as ready for review September 6, 2024 19:59
@MTRNord MTRNord requested a review from a team September 6, 2024 19:59
@MTRNord
Copy link
Contributor Author

MTRNord commented Sep 6, 2024

For discussion: We could replace rollup too with https://vitejs.dev/guide/build.html#library-mode

@MTRNord MTRNord mentioned this pull request Sep 9, 2024
5 tasks
@HarHarLinks
Copy link
Contributor

is there a reasonable way to sync certain configs across projects? e.g. I see parts of eslint config changed here to include the same options as standalone (but also more) and also options are in a different order.

@MTRNord
Copy link
Contributor Author

MTRNord commented Sep 17, 2024

is there a reasonable way to sync certain configs across projects? e.g. I see parts of eslint config changed here to include the same options as standalone (but also more) and also options are in a different order.

Not sure for vite/vitest, but for eslint we can make a ruleset and use that just like we do with other eslint plugins. There is already a ticket for this.

HarHarLinks

This comment was marked as resolved.

@HarHarLinks
Copy link
Contributor

Did/can you check if any of the warnings produces by yarn check are relevant to this PR?

@HarHarLinks

This comment was marked as resolved.

@HarHarLinks

This comment was marked as resolved.

@MTRNord

This comment was marked as resolved.

@MTRNord MTRNord force-pushed the nic/feat/NEO-681 branch 3 times, most recently from 92795de to 6cf1177 Compare September 23, 2024 19:47
@weeman1337 weeman1337 changed the title Migrate to vitejs + vitest Migrate to Vite + Vitest Sep 24, 2024
@weeman1337
Copy link
Contributor

is there a reasonable way to sync certain configs across projects? e.g. I see parts of eslint config changed here to include the same options as standalone (but also more) and also options are in a different order.

see NEO-689

@weeman1337

This comment was marked as resolved.

@weeman1337

This comment was marked as outdated.

@weeman1337

This comment was marked as outdated.

MTRNord and others added 2 commits September 24, 2024 14:16
Outline the vitest migration

Add changesets

Fix typos

Fix missing dependencies

Remove obsolete jest config

Remove more old references to jest

Use our own more simple vitest-axe extension

Adjust the i18next-parser.config.js

Ensure the coverage dependency is made a dependency and ensure we are not running in commonjs mode

Use the correct axe import

Fix vite config for the example widget and the dedup of it. Also fix the exports in the package jsons

Ensure we run cleanup from @testing-library/react after each test to have a clean state

Exclude the build and lib folders to prevent falsely detected tests

Run check-api-report to update the testing library

Ensure we don't mix act from different dependencies and just always use the one from react

Fixup review items and update docs as needed

Signed-off-by: MTRNord <[email protected]>
Signed-off-by: Michael Weimann <[email protected]>
Signed-off-by: Michael Weimann <[email protected]>
weeman1337

This comment was marked as outdated.

@HarHarLinks

This comment was marked as resolved.

Copy link
Contributor

@HarHarLinks HarHarLinks left a comment

Choose a reason for hiding this comment

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

please update containers/widget-server/README.md regarding CRA. there is another mention in the ADRs which I think we can keep.

@weeman1337

This comment was marked as resolved.

@weeman1337
Copy link
Contributor

weeman1337 commented Sep 25, 2024

should we change the default ports for vite in the neotoolsuite so the different things don't collide (or autoresolve the collision, mixing up the order every time)?

Vite should resolve port conflicts. You get an URL in the shell when running it. Will double-check anyway.

Can confirm. Vite looks for the next free port and prints it to the shell.

That isn't the point, I said I do not want to autoresolve the conflicts: I want to yarn dev the mui demo widget and add it to my test room at $port and if i tomorrow start the neoboard widget first and the demo widget second, the demo widget should still appear on $port so it is in the room i added it to and i don't need to set everything up from scratch again.

Ports for the example widget and preview are now: 5173 (http) and 5174 (https).

Lets use the next ports for neoboard etc. then.

If preview and dev should be different ports, leave a comment. I don't have a strong opinion on this.

Signed-off-by: Michael Weimann <[email protected]>
@weeman1337
Copy link
Contributor

please update containers/widget-server/README.md regarding CRA. there is another mention in the ADRs which I think we can keep.

done

weeman1337
weeman1337 previously approved these changes Sep 25, 2024
Signed-off-by: Michael Weimann <[email protected]>
@HarHarLinks HarHarLinks mentioned this pull request Sep 25, 2024
5 tasks
Signed-off-by: Michael Weimann <[email protected]>
Signed-off-by: Michael Weimann <[email protected]>
@weeman1337 weeman1337 merged commit f00e7cf into main Sep 26, 2024
4 checks passed
@weeman1337 weeman1337 deleted the nic/feat/NEO-681 branch September 26, 2024 11:34
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.

3 participants