-
Notifications
You must be signed in to change notification settings - Fork 982
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): Create uploads package with prisma extension and upload processors #11263
Conversation
Just need to make sure the type of compute in results extensions is correct
…rw-uploads * 'feat/rw-uploads' of github.com:dac09/redwood: chore(tsconfig.build): Fix type checking for tests etc (redwoodjs#11167) chore(config): migrate renovate config (redwoodjs#11157) fix(deps): update prisma monorepo to v5.18.0 (redwoodjs#11165) chore(fixture): Rebuild test project fixture (redwoodjs#11166) chore(ci): Print full diff when check-test-project-fixture fails (redwoodjs#11164) chore(rwjs/vite): Rename build script and format source (redwoodjs#11163) chore(tsconfig): Format vite tsconfig (redwoodjs#11162) RSC: RSA: Better match error handling with how we generally do it in RW (redwoodjs#11161) RSC: RSA: Add more comments to the basic 'use server' check (redwoodjs#11160) RSC: RSA: Remove unused check for '$$id' (redwoodjs#11159) RSC: Fix red squiggles in transform client tests (redwoodjs#11156) RSC: Improve type safety in RSC transform plugins (redwoodjs#11155)
…into feat/rw-uploads-extension * 'feat/rw-uploads-extension' of github.com:dac09/redwood: (22 commits) fix(jobs): Type-safe arguments passing to jobs from scheduler (redwoodjs#11371) chore(jobs tests): String literal array for queues (redwoodjs#11370) feat(jobs): Expose types (redwoodjs#11369) fix(template): Set scripts/ module resolution to match api/ (redwoodjs#11366) fix(template): Format tsconfig and jsconfig (redwoodjs#11365) fix(tsconfig): set "module" to "ESNext" for web/tsconfig.json (redwoodjs#11368) fix(cli): Fix tests generated by `g cell` (redwoodjs#11364) chore(deps): bump micromatch to 4.0.8 (redwoodjs#11362) chore(deps): bump micromatch to 4.0.8 in create-redwood-rsc-app (redwoodjs#11363) Adds docs for how to do recurring jobs (redwoodjs#11361) Adds docs for jobs setup and generate commands (redwoodjs#11359) feat(storybook): Remove consideration of mjs from docgen plugin (redwoodjs#11346) chore(deps): bump micromatch from 4.0.5 to 4.0.8 in /docs (redwoodjs#11358) chore(tarsync): Add watch mode to project:tarsync (redwoodjs#11347) chore(docs): Move content to main so we can deploy from that branch (redwoodjs#11356) chore(jobs test): Remove unused file. Use toContain() (redwoodjs#11355) chore(listr2mock): Move comments closer to referenced code (redwoodjs#11354) chore(listr2mock): Improve types and make it more capable (redwoodjs#11352) chore(jobs): Add e2e test script and CI job (redwoodjs#11348) chore(cli tests): Fix include glob (redwoodjs#11351) ...
✋ HODL-ing this till @cannikin has had a chance to go over! |
// @ts-expect-error TS in strict mode will error due to union type. We cannot narrow it down here. | ||
await prismaInstance[model as ModelNames].findFirstOrThrow(args) | ||
|
||
await removeUploadedFiles(uploadFields, record) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this do the same thing as update
above: run the query first, and only if the query succeeds do you delete the file? I could see a delete failing if there are foreign key constraints...but maybe Prisma handles that check elsewhere and so this code wouldn't even be called if that happened?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah you're right... I'm sure I added the test case for this too, but maybe i messed up git. I'll fix it1 Thanks!!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated this now, but the weird thing is.... I tried it without my changes too, and it seems prisma does handle the check before it reaches the extension.
Regardless I managed to remove one extra call for findFirstOrThrow
because delete does return the deleted record, which we need to know what paths to delete!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Dude this is amazing!
Only other comment I had was for file organization: in the jobs code we put the adapters in their own src/adapters dir and then each adapter got its own dir so auxiliary stuff (and tests) could be located together. They're all still exported at the top level of the package though, you don't have to do like @redwoodjs/jobs/adapters
to get to them, still just @redwoodjs/jobs
. I'm trying to think if we have any other packages that do that...but if we get a chance, we should make them consistent I think!
Good idea, and I'll do this when I move the files over to the
Thanks yo!! I'll actually move the storage stuff into a separate package I think. So the imports will look like:
The reason I'm leaning towards specific imports is so we can avoid having big index.js files -and in v8 we're officially introducing support for this - and is sort of the ESM way. This should become the new standard as we move towards the ESM world and being able to use package.json exports. But let's go over the paths in the next PR when I move the files over |
@Josh-Walker-GM just a headsup that this should not be released in v7.x, but only in v8+! Because we're in that gray zone of "we're about to release", I've tagged it as next-release. Ofcourse the full uploads feature won't actually get released until 8.1 I imagine (depending on when the release goes out). |
…ads-extension-extra-crud * 'main' of github.com:redwoodjs/redwood: feat(uploads): Create uploads package with prisma extension and upload processors (redwoodjs#11263) chore(test): Update tests to make them compatible with Vite's ecosystem CI (redwoodjs#11384) Catch errors when studio is not running and mail tries to send (redwoodjs#11387) template(db): Update `api/src/lib/db` template (redwoodjs#11379) feat(uploads): Add File scalar to rootSchema (redwoodjs#11378) fix(jobs): Fix the `deleteSuccessfulJobs` type showing up (redwoodjs#11377) RCS: Basic support for 'use server' inside components (redwoodjs#11376) chore(deps): Add missing dev dep (memfs) to our vite package (redwoodjs#11373) fix(jobs): Allow passing empty array when scheduling parameterless jobs (redwoodjs#11372)
The API side for Redwood Uploads, that includes:
a) Query Extension: will save, delete, replace files on disk during CRUD
b) Result Extension: gives you functions like
.withSignedUri
on configured prisma results - which will take the paths, and convert it to a signed urlFiles
and save them to storageThere's a lot of details in the readme! https://github.com/redwoodjs/redwood/pull/11263/files#diff-fa3b71228abef71ce50bf2b73f86488541db4ee01e2082172a069036ca43a4a2