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

[ci skip] try Lachlan's vite plugin #103

Merged
merged 3 commits into from
Apr 21, 2023

Conversation

muratkeremozcan
Copy link
Owner

@muratkeremozcan muratkeremozcan commented Apr 19, 2023

image

@muratkeremozcan muratkeremozcan marked this pull request as draft April 19, 2023 12:12
@@ -42,17 +42,18 @@
"react-icons": "4.8.0",
"react-query": "3.39.3",
"react-router-dom": "6.10.0",
"react-scripts": "5.0.1",
Copy link
Owner Author

Choose a reason for hiding this comment

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

realized I left this here before...

vite.config.ts Outdated
@@ -3,6 +3,7 @@ import path from 'path'
import react from '@vitejs/plugin-react' // cy code cov
import istanbul from 'vite-plugin-istanbul' // cy code cov
import EnvironmentPlugin from 'vite-plugin-environment' // to be able to use process.env.
import {CypressMocks} from '@lmiller1990/vite-plugin-proxify-esm'
Copy link
Owner Author

Choose a reason for hiding this comment

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

@lmiller1990 is this how we use it?

@lmiller1990
Copy link
Collaborator

lmiller1990 commented Apr 20, 2023

Looks like we have some bugs still. I fixed this one, but now:

image

For reference, fix: cypress-io/cypress@befd9dc#diff-22e277a2300647e6ce61bd2e48e980556314105f2c7b5c47306d1e25d1412411R57

src/About.tsx Outdated
consistent in their styles and design decisions. This one inspires from
them, uses CCTDD and takes variances along the way.
</p>
function About() {
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm not sure why, but this was giving me issues. I had to change from const to function and it works.

I think React Router does something unusual under-the-hood with lazy, I am not sure what, but I'll look into it. Using const with the plugin causes a very weird error.

More info: https://github.com/cypress-io/cypress/blob/6a4665cf49819714b40f7c0a3aa17f8e97442da2/npm/vite-plugin-cypress-esm/README.md#react-router-and-lazy-routes-issue

Copy link
Owner Author

Choose a reason for hiding this comment

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

It's better to use named functions for stack trace anyway, but it would be nice if at work we didn't have to change a few hundred of them from anonymous to named.

@@ -16,6 +17,9 @@ export default defineConfig({
requireEnv: false,
}),
EnvironmentPlugin(['VITE_API_URL']),
CypressEsm({
ignoreList: ['*react*'],
Copy link
Collaborator

Choose a reason for hiding this comment

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

Info on ignoreList in the readme: https://github.com/cypress-io/cypress/blob/6a4665cf49819714b40f7c0a3aa17f8e97442da2/npm/vite-plugin-cypress-esm/README.md#react-router-and-lazy-routes-issue

Basically we wrap all modules in a Proxy to allow Sinon to stub/spy. Some libraries do not seem to play nicely with Proxy. Also, it can incur some performance overhead. This opts out of the Proxy ES module runtime. I'm thinking you don't generally want to stub things like your code modules anyway, like Router etc - so I think this is okay.

Long term, we will make sure all the modules work as expected, so this won't be needed.

@cypress
Copy link

cypress bot commented Apr 21, 2023

Passing run #579 ↗︎

0 96 0 0 Flakiness 0

Details:

enabled the commented out spies
Project: Tour of Heroes CCTDD - Vite Commit: 873c3b332f
Status: Passed Duration: 02:26 💡
Started: Apr 21, 2023 11:50 AM Ended: Apr 21, 2023 11:53 AM

This comment has been generated by cypress-bot as a result of this project's GitHub integration settings.

@codecov
Copy link

codecov bot commented Apr 21, 2023

Codecov Report

Patch coverage: 100.00% and project coverage change: +0.18 🎉

Comparison is base (0ae1160) 96.70% compared to head (873c3b3) 96.89%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #103      +/-   ##
==========================================
+ Coverage   96.70%   96.89%   +0.18%     
==========================================
  Files          33       33              
  Lines         516      515       -1     
  Branches       67       66       -1     
==========================================
  Hits          499      499              
+ Misses         17       16       -1     
Flag Coverage Δ
cypress-ct-coverage 93.83% <100.00%> (ø)
cypress-e2e-coverage 93.35% <100.00%> (ø)
jest-rtl-coverage 86.39% <0.00%> (+0.17%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
src/About.tsx 100.00% <100.00%> (+25.00%) ⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@muratkeremozcan muratkeremozcan marked this pull request as ready for review April 21, 2023 11:54
@muratkeremozcan muratkeremozcan merged commit ae3e5e7 into main Apr 21, 2023
@lmiller1990 lmiller1990 deleted the test/vite-plugin-proxify-esm branch April 23, 2023 22:39
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