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

Add packages/wpcom-proxy-request. #39139

Merged
merged 115 commits into from
Feb 18, 2020
Merged

Add packages/wpcom-proxy-request. #39139

merged 115 commits into from
Feb 18, 2020

Conversation

sgomes
Copy link
Contributor

@sgomes sgomes commented Jan 29, 2020

This PR deals with importing the source code (and history) only, while keeping it inactive. Subsequent PRs will build on top of this to modernise the code and integrate it with calypso-build.

Marking the package as private to avoid publishing for now.

Note: the intention is to merge this PR as a merge commit, following what was done with wpcom. This accurately reflects the fact that we are merging the histories of two projects.

Changes proposed in this Pull Request

  • Import source code for wpcom-proxy-request.
  • Mark package as private.

Testing instructions

None of this code is active, so it should be sufficient to test that the build does not break.

@sgomes sgomes added [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. Packages labels Jan 29, 2020
@sgomes sgomes requested review from jsnajdr and a team January 29, 2020 14:35
@matticbot
Copy link
Contributor

@matticbot
Copy link
Contributor

This PR does not affect the size of JS and CSS bundles shipped to the user's browser.

Generated by performance advisor bot at iscalypsofastyet.com.

@blowery blowery mentioned this pull request Jan 29, 2020
14 tasks
@sgomes
Copy link
Contributor Author

sgomes commented Feb 4, 2020

Any chance of a quick review for this one? All it does is import the source and history for wpcom-proxy-request into the monorepo (like #39080 did for wpcom), and it would unlock further work in making builds smaller.

@sgomes
Copy link
Contributor Author

sgomes commented Feb 17, 2020

Friendly ping. All this PR does is import the existing source and history for wpcom-proxy-request to unlock further work. The code will stay inert until it's activated in a later PR.

retrofox and others added 21 commits February 18, 2020 13:39
Improve error handling and WP-API integration
* First pass at allowing reloading

* Build after installing, so we can use GitHub branches as dependencies

* Use npm instead of make for postinstall

* Try using postinstall-build for postinstall?

* Package: need to include index.js, so it can be built.

* Package: need to include Makefile, too.

* Wait for a message from the iframe to trigger onload()

* Run the test suite twice - once normally, then once after reload.

* Add myself as a contributor

* Fix some code formatting bugs

* Let prepublish build the lib when installed as a dependency

* Bump package version to v4.0.0

* Don't add index.js and Makefile to npm package
* Fix: Stop treating completely failed network requests as successes

Resolves #5

When certain network requests fail because the internet connection is completely broken they are coming through as if they were successes and the passed callback is called with `(err = null, data = {})`. This breaks the application logic which depends on failing requests actually failing.

I added the strict-equality check on the returned `statusCode` because I noticed that when the network connection was severed it was coming back with a value of `0` and not `null`. This appears to fix the problem I'm observing.

On the other hand, it's not clear to me when the status would be `null` or why this check is in there to begin with. I'm not sure what other things could be breaking as a result of this change. Would really appreciate some review and some insight into this odd behavior.

Thanks!

* Only accept 2XX status codes as successful requests

* pkg: bump to 4.0.0-alpha.1 version

* Bump package version to v4.0.1
* Detect metaAPI responses and simplify ready event

* Bump package version to 4.0.2
* We arent passed with the envelope

* Treat statusCode as always exists

* Bump package version to 4.0.5
Unbind window.message when uninstalling
Bump to 4.0.6 and add blowery to contribs
…n of the package

I'm trying to finish the long-awaited project to slim down the REST Proxy by removing
the dependency on jQuery. The new version will be available at:
```
https://public-api.wordpress.com/wp-admin/rest-proxy/?v=2.0
```
This patch updates the `src` of the iframe that this module will load to the new URL.

We'll release this version of the package as 5.0.0, since there is quite major
update of the server-side implementation (although its behavior is supposed to be unchanged).
)

* Patch File objects sent as form data to work around a bug in Chrome

Before sending request data over `window.postMessage` to the proxy iframe, we find all `File`
instances in the form data and create a new `File` instance based on the existing `File`. That
forces the `File` object's storage to be a `Blob` instead of being backed by a file on disk.
That works around a bug in Chrome where `File` instances with `has_backing_file` flag cannot
be sent over a process boundary when site isolation is on.

This PR also removes an old workaround for Firefox where the browser couldn't send a `File` over
`window.postMessage` at all. That bug has been fixed for several years now.

* Feature detect File constructor on IE and Edge

* Update package-lock.json
* Fix Chrome file upload workaround to avoid breaking Safari 10

The `new File( ... )` trick to patch `File` object breaks file uploads on Safari 10.
Safari then uploads empty files. This PR limits the patching to Chrome.

* Improve detection of Chrome to avoid breaking file uploads on Edge

Edge also exposes `window.chrome`, so we need to detect `File` constructor after all...
This PR deals with importing the source code only, while keeping it
inactive. Subsequent PRs will build on top of this.

Marking the package as private to avoid publishing for now.
@sgomes sgomes force-pushed the packages/wpcom-proxy-request branch from f63d1d5 to 4d956af Compare February 18, 2020 13:40
Copy link
Member

@tyxla tyxla left a comment

Choose a reason for hiding this comment

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

LGTM 👍 Excited to see what's coming for this one.

@tyxla tyxla added [Status] Ready to Merge and removed [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. labels Feb 18, 2020
@sgomes
Copy link
Contributor Author

sgomes commented Feb 18, 2020

Thank you so much for the review, @tyxla! 🙏

@sgomes sgomes merged commit ba5a974 into master Feb 18, 2020
@sgomes sgomes deleted the packages/wpcom-proxy-request branch February 18, 2020 13:59
@sgomes
Copy link
Contributor Author

sgomes commented Feb 18, 2020

Merged with a merge commit, to preserve the history of the imported repository.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.