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

Refactor proxy into own package, implement middleware pattern #5136

Merged
merged 27 commits into from
Nov 28, 2019

Conversation

flotwig
Copy link
Contributor

@flotwig flotwig commented Sep 12, 2019

  • Moved proxy.coffee and other related files from the server into a new proxy package.
  • proxy.coffee has been split up into Request, Response, and Error middleware stacks, which are now easier to unit test.
  • request.coffee has been converted to Typescript and moved to the network package along with associated tests.
    • Pending Electron v5.0.10 #4720 - request.coffee was changed a lot and it would be wasted effort to refactor it from develop
    • Not necessary for now
  • All modified/moved code has been converted to Typescript

Pre-merge Tasks

  • Have tests been added/updated for the changes in this PR?
    • Added unit tests for proxy layers
    • Moved tests for utilities that were moved to network, proxy
  • Has the original issue been tagged with a release in ZenHub?

@cypress
Copy link

cypress bot commented Sep 12, 2019



Test summary

3515 0 45 0


Run details

Project cypress
Status Passed
Commit 27edf34
Started Nov 26, 2019 7:43 PM
Ended Nov 26, 2019 7:47 PM
Duration 03:59 💡
OS Linux Debian - 9.8
Browser Multiple

View run in Cypress Dashboard ➡️


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

don't need these mocha opts anymore

fix test

no more zunder

READMEs

fix test
@flotwig flotwig changed the title [WIP] Refactor proxy into own package, implement middleware pattern Refactor proxy into own package, implement middleware pattern Oct 10, 2019
packages/network/README.md Show resolved Hide resolved
packages/network/lib/cors.ts Show resolved Hide resolved
packages/network/lib/uri.ts Outdated Show resolved Hide resolved
packages/proxy/README.md Show resolved Hide resolved
packages/proxy/lib/network-proxy.ts Show resolved Hide resolved
packages/proxy/package.json Outdated Show resolved Hide resolved
packages/server/lib/controllers/proxy.coffee Show resolved Hide resolved
packages/server/package.json Show resolved Hide resolved
@brian-mann brian-mann self-requested a review October 13, 2019 18:59
Copy link
Member

@brian-mann brian-mann left a comment

Choose a reason for hiding this comment

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

Whenever you decaf stuff - submit those files as a separate PR outside of this so we can avoid the squash and merge.

@flotwig
Copy link
Contributor Author

flotwig commented Oct 15, 2019

@brian-mann All decaffing was done in a separate PR: #5156

However, this PR moves to new packages + edits some files, so not sure if that will break Git history or what.

@flotwig flotwig dismissed stale reviews from jennifer-shehane and brian-mann October 17, 2019 20:48

responded

@jennifer-shehane jennifer-shehane requested a review from a team October 18, 2019 21:18
@flotwig
Copy link
Contributor Author

flotwig commented Nov 26, 2019

@brian-mann @jennifer-shehane Is this good to merge in? Just brought it back up to date with develop.

I believe all the feedback that was left previously has been addressed.

Since this forms the basis for #687 I'd like to get it merged in develop sooner rather than later.

@flotwig flotwig merged commit b0378dc into develop Nov 28, 2019
avallete pushed a commit to avallete/cypress that referenced this pull request Nov 28, 2019
…s-io#5136)

* renames

* Refactor proxy into own package, implement middleware pattern

don't need these mocha opts anymore

fix test

no more zunder

READMEs

fix test

* pass request by reference

* fix cors path

* Move replace_stream to proxy, concat-stream util in network

* Pin dependency versions

* Revert addDefaultPort behavior

* Add READMEs for proxy, network

* Update README.md

* eslint --fix

* set to null not undefined

* use delete and bump node types

* import cors from package now

* parse-domain@2.3.4

* proxy package needs common-tags

* move pumpify dep

* load through where it's needed, remove unused passthru_stream

* remove unneeded getbuffer call


Co-authored-by: Gleb Bahmutov <gleb.bahmutov@gmail.com>
santoshyadavdev pushed a commit to santoshyadavdev/cypress that referenced this pull request Dec 5, 2019
…s-io#5136)

* renames

* Refactor proxy into own package, implement middleware pattern

don't need these mocha opts anymore

fix test

no more zunder

READMEs

fix test

* pass request by reference

* fix cors path

* Move replace_stream to proxy, concat-stream util in network

* Pin dependency versions

* Revert addDefaultPort behavior

* Add READMEs for proxy, network

* Update README.md

* eslint --fix

* set to null not undefined

* use delete and bump node types

* import cors from package now

* parse-domain@2.3.4

* proxy package needs common-tags

* move pumpify dep

* load through where it's needed, remove unused passthru_stream

* remove unneeded getbuffer call


Co-authored-by: Gleb Bahmutov <gleb.bahmutov@gmail.com>
@cypress-bot
Copy link
Contributor

cypress-bot bot commented Dec 12, 2019

Released in 3.8.0.

@flotwig flotwig deleted the refactor-proxy branch January 24, 2022 18:18
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.

4 participants