fix uploadRemoteFile undefined #4814
Conversation
for some reason, only when testing locally using VITE_COMPANION_URL=https://api2.transloadit.com/companion only we get this error... i'm thinking for some reason, the remote file gets serialized into state and then de-serialized again causing remote.requestClient to be gone so I solve it by instead storing the request clients in Uppy fixes #4791
packages/@uppy/url/src/Url.jsx
Outdated
| @@ -88,6 +88,9 @@ export default class Url extends UIPlugin { | |||
| companionHeaders: this.opts.companionHeaders, | |||
| companionCookiesRule: this.opts.companionCookiesRule, | |||
| }) | |||
|
|
|||
| this.requestClientId = 'url'; | |||
There was a problem hiding this comment.
What about reusing this.id? And why are we only setting this for the url plugin, not all other providers?
There was a problem hiding this comment.
it's a bit hard to see between all the whitespace changes, but i also did the same in View.js (used by providers). as for reusing this.id, it feels a bit error prone, if someone customizes their id, then it could blow up
There was a problem hiding this comment.
But they could also customize requestClientId, no? Should we use .constructor.name instead? That seems unlikely to change
There was a problem hiding this comment.
ok have now refactored to use constructor.name, but in View.js we have to use this.provider.provider because we cannot get constructor.name
|
Not sure how it’s best implemented TBH, but yea I agree it’s a bit strange to do it like this. |
|
argh suddenly the PR has a ton of whitespace changes. i didn't commit this. i think there's something wrong with our prettier setup |
| @@ -88,6 +91,8 @@ export default class Url extends UIPlugin { | |||
| companionHeaders: this.opts.companionHeaders, | |||
| companionCookiesRule: this.opts.companionCookiesRule, | |||
| }) | |||
|
|
|||
| this.uppy.registerRequestClient(Url.requestClientId, this.client) | |||
There was a problem hiding this comment.
Why only Url and not all other providers?
There was a problem hiding this comment.
not only url, see also changes in ProviderView and SearchProviderView
There was a problem hiding this comment.
Can we add a comment then why URL is the only exception?
There was a problem hiding this comment.
No, what i'm saying is that URL is not an exception. we do the same inside ProviderView and SearchProviderView
There was a problem hiding this comment.
It is an exception because URL is apparently the only one that needs to have it explicitly, while the rest of the plugins have it implicitly through ProviderView?
There was a problem hiding this comment.
Hmm, not sure what the comment would say.
| /** | ||
| * Url | ||
| * | ||
| */ | ||
| export default class Url extends UIPlugin { | ||
| static VERSION = packageJson.version | ||
|
|
||
| static requestClientId = Url.name |
There was a problem hiding this comment.
nit
| static requestClientId = Url.name | |
| static requestClientId = this.name |
There was a problem hiding this comment.
You sure that will work? doesn't this imply we have an instance? static happens before we have an instance right?
| Package | Version | Package | Version | | ---------------------- | ------- | ---------------------- | ------- | | @uppy/aws-s3 | 3.6.0 | @uppy/instagram | 3.2.0 | | @uppy/aws-s3-multipart | 3.10.0 | @uppy/onedrive | 3.2.0 | | @uppy/box | 2.2.0 | @uppy/provider-views | 3.8.0 | | @uppy/companion | 4.12.0 | @uppy/store-default | 3.2.0 | | @uppy/companion-client | 3.7.0 | @uppy/tus | 3.5.0 | | @uppy/core | 3.8.0 | @uppy/url | 3.5.0 | | @uppy/dropbox | 3.2.0 | @uppy/utils | 5.7.0 | | @uppy/facebook | 3.2.0 | @uppy/xhr-upload | 3.6.0 | | @uppy/google-drive | 3.4.0 | @uppy/zoom | 2.2.0 | | @uppy/image-editor | 2.4.0 | uppy | 3.21.0 | - @uppy/provider-views: fix uploadRemoteFile undefined (Mikael Finstad / #4814) - @uppy/companion: fix double tus uploads (Mikael Finstad / #4816) - @uppy/companion: fix accelerated endpoints for presigned POST (Mikael Finstad / #4817) - @uppy/companion: fix `authProvider` property inconsistency (Mikael Finstad / #4672) - @uppy/companion: send certain onedrive errors to the user (Mikael Finstad / #4671) - meta: fix typo in `lockfile_check.yml` name (Antoine du Hamel) - @uppy/aws-s3: change Companion URL in tests (Antoine du Hamel) - @uppy/set-state: fix types (Antoine du Hamel) - @uppy/companion: Provider user sessions (Mikael Finstad / #4619) - meta: fix `js2ts` script on Node.js 20+ (Merlijn Vos / #4802) - @uppy/companion-client: avoid unnecessary preflight requests (Antoine du Hamel / #4462) - meta: Migrate to AWS-SDK V3 syntax (Artur Paikin / #4810) - @uppy/utils: fix import in test files (Antoine du Hamel / #4806) - @uppy/core: Fix onBeforeFileAdded with Golden Retriever (Merlijn Vos / #4799) - @uppy/image-editor: respect `cropperOptions.initialAspectRatio` (Lucklj521 / #4805)
for some reason, only when testing locally using VITE_COMPANION_URL=https://api2.transloadit.com/companion only we get this error...
i'm thinking for some reason, the remote file gets serialized into state and then de-serialized again causing remote.requestClient to be gone
so I solve it by instead storing the request clients in Uppy
fixes #4791