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(uploads): Configure apollo client to do multi-part form uploads #11175

Merged
merged 16 commits into from
Aug 19, 2024

Conversation

dac09
Copy link
Contributor

@dac09 dac09 commented Aug 9, 2024

This PR does the following:

a) Configures the Apollo client we export to use upload link - https://github.com/jaydenseric/apollo-upload-client
b) Configures our API side fastify server to accept multipart form data
c) Generates types for File scalars in graphql codegen

Notes:

  1. apollo-upload-client is ESM only. In order to get this working for prerender, I had to bundle it for CJS version only.
    Without this change you get errors during prerender like this:
Error [ERR_REQUIRE_ESM]: require() of ES Module /Users/dac09/Experiments/apollo-upload-link/node_modules/apollo-upload-client/createUploadLink.mjs not supported.
  1. Currently the multi-part config only applies when you have a server file (see separate PR with fix: fix(api-server): Use createServer in all cases, to make fastify config consistent #11176)

  2. The upload link internally will handle whether to do a regular POST or multipart POST. In order to make use of this on the backend you need to set your graphql schema field to a scalar of File

@dac09 dac09 added this to the next-release milestone Aug 9, 2024
@dac09 dac09 added the release:feature This PR introduces a new feature label Aug 9, 2024
@dac09 dac09 marked this pull request as ready for review August 9, 2024 11:22
dac09 added 7 commits August 9, 2024 18:25
…pload-link

* 'main' of github.com:redwoodjs/redwood: (56 commits)
  chore(deps): update babel monorepo (redwoodjs#11190)
  fix(deps): update dependency jscodeshift to v17 (redwoodjs#11233)
  chore(deps): update babel monorepo  (redwoodjs#11228)
  chore(deps): update dependency @clerk/types to v3.65.3 (redwoodjs#11230)
  fix(deps): update dependency vite-plugin-cjs-interop to v2.1.2 (redwoodjs#11224)
  fix(deps): update graphql-tools monorepo (redwoodjs#11232)
  chore(deps): update dependency @clerk/clerk-react to v4.32.3 (redwoodjs#11229)
  fix(deps): update dependency @clerk/clerk-sdk-node to v4.13.21 (redwoodjs#11231)
  fix(deps): update dependency cheerio to v1.0.0 (redwoodjs#11218)
  chore(deps): update yarn to v4.4.0 (redwoodjs#11227)
  fix(deps): update dependency core-js to v3.38.0 (redwoodjs#11226)
  chore(deps): update eslint monorepo to v9.9.0 (redwoodjs#11202)
  fix(api-server): Use createServer in all cases, to make fastify config consistent (redwoodjs#11176)
  chore(deps): update dependency tsx to v4.17.0 (redwoodjs#11199)
  chore(deps): update dependency @types/vscode to v1.92.0 (redwoodjs#11193)
  fix(deps): update dependency @graphql-yoga/plugin-defer-stream to v3.6.3 (redwoodjs#11211)
  fix(deps): update dependency @graphql-yoga/plugin-persisted-operations to v3.6.3 (redwoodjs#11213)
  chore(deps): update dependency eslint-plugin-perfectionist to v3.1.3 (redwoodjs#11195)
  chore(deps): update dependency eslint-plugin-jsdoc to v48.11.0 (redwoodjs#11194)
  chore(deps): update dependency @playwright/test to v1.46.0 (redwoodjs#11191)
  ...
…pload-link

* 'main' of github.com:redwoodjs/redwood:
  chore(linting): Remove/fix references to non-existant files (redwoodjs#11245)
  chore(rsa): Use swc for parsing server actions (redwoodjs#11243)
  chore(lint): Remove override for 'unused-imports/no-unused-imports' (redwoodjs#11244)
  chore(linting): Separate out framework and user linting config (redwoodjs#11242)
  fix: Update default tsconfig options (target, module and moduleResolution) (redwoodjs#11170)
  chore(fixture): Update tailwind dep (redwoodjs#11241)
  chore(deps): bump fast-xml-parser from 4.4.0 to 4.4.1 (redwoodjs#11239)
  chore(rsc): Switch last remaining transform-server test to inline snapshot (redwoodjs#11240)
  chore: brought in typescript-eslint@v8 with stylistic preset (redwoodjs#10911)
  chore(deps): bump axios from 1.7.3 to 1.7.4 (redwoodjs#11237)
  docs(serverConfig): Remove server config option from TOML (redwoodjs#11236)
  fix(deps): update typescript-eslint monorepo to v8 (major) (redwoodjs#11235)
…o try/apollo-upload-link

* 'try/apollo-upload-link' of github.com:dac09/redwood:
  chore(linting): enable type checked linting (redwoodjs#11258)
  chore(lint): lint config refactoring and re-enable some stylistic rules (redwoodjs#11257)
  chore(linting): Re-enable some ts-eslint rules (redwoodjs#11256)
  feat(rsc): Register top-level function-scoped RSAs (redwoodjs#11255)
  chore(lint): refactor react, react-hooks and ts-eslint usage (redwoodjs#11254)
  chore(formatting): Add prettier check to CI (redwoodjs#11253)
  chore(formatting): Formatting 2 of n (redwoodjs#11252)
  chore(formatting): Formatting 1 of n (redwoodjs#11251)
  chore(formatting): Format readme files (redwoodjs#11248)
  chore(record): Remove used file  (redwoodjs#11247)
  chore(formatting): Remove 'insert_final_newline' from editor config (redwoodjs#11249)
  chore(lint): Split linting and formatting (redwoodjs#11246)
Copy link
Collaborator

@Josh-Walker-GM Josh-Walker-GM left a comment

Choose a reason for hiding this comment

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

Reading over and discussing everything with you it all seems perfectly reasonable given some of the challenges that came up.

packages/api-server/package.json Outdated Show resolved Hide resolved
Comment on lines 36 to 38
// @MARK: We need to disable this in order for multipart requests to work
// otherwise you get incomprehensible errors like: 'Missing multipart form field "operations"'
// await fastify.register(fastifyRawBody)
Copy link
Collaborator

Choose a reason for hiding this comment

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

We spoke about this. I can't remember the history of this exactly but I think it was to do with the graphql server no longer being served by the api plugin and was now it's it's own plugin - with it's own set of plugins.

For now we should keep going and not block this PR on this. I'm hoping to setup smoke tests with the server file setup now that it's more first class than it used to be.

Copy link
Contributor Author

@dac09 dac09 Aug 19, 2024

Choose a reason for hiding this comment

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

I get a feeling it's historical, and speaking to DT, I couldn't find a reason we need this in the graphql plugin - I suspect it was originally intended for the functions plugin (which used to have shared settings like you said)

dac09 added 3 commits August 19, 2024 10:13
…pload-link

* 'main' of github.com:redwoodjs/redwood: (52 commits)
  linting: enable 'typescript-eslint/await-thenable' rule (redwoodjs#11311)
  refactor(api): Add conditional exports to package.json (redwoodjs#11307)
  chore(readme): Add my middle initials (redwoodjs#11310)
  chore(README): Fix formatting. One row (redwoodjs#11309)
  chore(README): Move Kris from Maintainer to Alumni (redwoodjs#11308)
  fix(codemods): Move away from babel for building package (redwoodjs#11306)
  fix(api): Move away from babel for building package (redwoodjs#11305)
  fix(internal): Move away from babel for building package (redwoodjs#11304)
  fix(auth-providers): Move away from babel for building 'api' packages (redwoodjs#11301)
  fix(auth-providers): Move away from babel for building 'setup' packages  (redwoodjs#11303)
  chore(deps): Remove webpack related dependencies (redwoodjs#11299)
  chore(build): build core with esbuild (redwoodjs#11298)
  chore(exec test): Clean up (redwoodjs#11302)
  Detect/resolve ambiguous script names (redwoodjs#9848)
  chore(lint): tidy up the prettier ignore (redwoodjs#11297)
  feat(context): Build and publish package as dual esm/cjs (redwoodjs#11294)
  chore(lint): fix prettier configs and ignores (redwoodjs#11295)
  chore(docs): update prettier config and format docs content (redwoodjs#11293)
  chore(check): Refactor 'yarn check' away from being a standalone node script  (redwoodjs#11292)
  chore: delete crowdin config file (redwoodjs#11291)
  ...
@dac09
Copy link
Contributor Author

dac09 commented Aug 19, 2024

Marking this PR as next-release-major, because it doesn't work smoothly unless you're using a server file. (Tied with #11176)

@dac09 dac09 merged commit 75cc37c into redwoodjs:main Aug 19, 2024
47 checks passed
@dac09 dac09 deleted the try/apollo-upload-link branch August 19, 2024 06:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release:feature This PR introduces a new feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants