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

feat: normalized signatures webpack & vite servers #18379

Merged
merged 7 commits into from
Oct 14, 2021

Conversation

elevatebart
Copy link
Contributor

@elevatebart elevatebart commented Oct 6, 2021

User facing changelog

In preparation for new plugins syntax, add the needed signatures to vite and webpack dev server

Additional details

https://www.notion.so/cypressdx/Dev-server-module-signatures-exports-db9cf2518a1b485a9672811f3e895de7

@cypress-bot
Copy link
Contributor

cypress-bot bot commented Oct 6, 2021

Thanks for taking the time to open a PR!

@elevatebart elevatebart changed the title chore: release @cypress/react-v5.10.1 @elevatebart feat: normalized signatures webpack & vite servers Oct 6, 2021
@elevatebart elevatebart changed the title @elevatebart feat: normalized signatures webpack & vite servers feat: normalized signatures webpack & vite servers Oct 6, 2021
@cypress
Copy link

cypress bot commented Oct 6, 2021



Test summary

18544 0 214 7Flakiness 2


Run details

Project cypress
Status Passed
Commit 497ec2c
Started Oct 12, 2021 8:21 PM
Ended Oct 12, 2021 8:34 PM
Duration 12:52 💡
OS Linux Debian - 10.9
Browser Multiple

View run in Cypress Dashboard ➡️


Flakiness

cypress/proxy-logging-spec.ts Flakiness
1 ... > works with forceNetworkError
e2e/redirects_spec.js Flakiness
1 redirection > meta > binds to the new page after a timeout

This comment has been generated by cypress-bot as a result of this project's GitHub integration settings. You can manage this integration in this project's settings in the Cypress Dashboard

Copy link
Contributor

@cowboy cowboy left a comment

Choose a reason for hiding this comment

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

I'd like to see one minor change. Also, this PR is missing tests. Can you add them?

npm/vite-dev-server/src/index.ts Outdated Show resolved Hide resolved
@cowboy
Copy link
Contributor

cowboy commented Oct 7, 2021

@elevatebart what is the plan for testing these changes? It would be good to get them merged in, but I'm hesitant to approve the PR if there are no tests being added to assert the new changes are correct.

@elevatebart
Copy link
Contributor Author

@cowboy I decided to add some tests after all.
They will be hard to justify in the future but they are easy to delete.

  • in a33383b I added a test for vite-dev-server
  • in d283dd9 I added them for webpack-dev-server

I hope this helps you approve of this change.
Keep me posted if you have any Qs

Copy link
Contributor

@cowboy cowboy left a comment

Choose a reason for hiding this comment

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

I don't really understand the tests, but as long as they give you confidence, 👍🏻 . Only a few other questions to be answered or addressed.

npm/webpack-dev-server/package.json Outdated Show resolved Hide resolved
npm/webpack-dev-server/src/startServer.ts Show resolved Hide resolved
npm/webpack-dev-server/src/startServer.ts Outdated Show resolved Hide resolved
@elevatebart elevatebart changed the title feat: normalized signatures webpack & vite servers feat: normalized signatures webpack & vite servers Oct 14, 2021
@elevatebart elevatebart changed the title feat: normalized signatures webpack & vite servers feat: normalized signatures webpack & vite servers Oct 14, 2021
@elevatebart elevatebart merged commit 8f5308f into master Oct 14, 2021
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.

None yet

2 participants