-
Notifications
You must be signed in to change notification settings - Fork 558
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 module upload support #1010
add module upload support #1010
Conversation
changelog detected ✅ |
0cddf4c
to
b0df422
Compare
Awesome! Don't say I never shopped on Shopify :) Since I'm just working on a round-trip use case over in #1007, I'll note that
The same API call for a Module-format worker returns multipart that would need to be parsed. What should happen if there are multiple parts?
The return type from
But the code for the method actually does this:
I'm not sure I follow what's going on there with how Go is inferring and passing types from that method (limitations of my Go knowledge) |
Got it, the struct fields are being promoted from nested structs
Thing is, most of these fields are not set by the So back to the issue at hand: is it possible for a DownloadWorker to respond with multiple module files? If so, what's the best way to represent that as a Go structure? (I'm thinking |
That is an excellent question. I'm not really sure as I haven't tried that/needed to do that yet. I believe that the upload functionality at least is covered here, so if download could be implemented separately, this should be good to go as is (at least it has been working for us so far). |
@jgodson i'm hoping to getting to review/test this in the coming days. are you able to address the CHANGELOG entry so there is one less thing to do before we merge? |
thanks for your patience here @jgodson. the PR looks great and my (basic) testing of this internally is looking great. |
@jacobbednarz Glad to see this land! Where are you at in terms of thinking about how to solve for DownloadWorker to return something sensible when a module worker is fetched? #1010 (comment) |
haven't really dug into the issues with |
This functionality has been released in v0.47.0. For further feature requests or bug reports with this functionality, please create a new GitHub issue following the template. Thank you! |
PR to upstream the changes from Shopify#16 that adds support for uploading scripts that use the module syntax.
It builds upon the changes started here
Closes #794