type-graphql
type-graphql copied to clipboard
Make it work on edge functions
Is your feature request related to a problem? Please describe.
typegraphql
cannot work, at the moment, on edge function engines, like the one of cloudflare workers or vercel@edge. The only reason why it cannot work is because the "emit schema to file" feature relies on path
and fs
libraries. No other part of the code is problematic. This is a little shame because a secondary feature is preventing it to work.
Describe the solution you'd like
Since the only feature is emitSchemaFile
we could isolate this functionality into a different package (e.g. typegraphql-schema-emit
or as submodule typegraphql/schema-emit
) and have it as optional peer dependency, so that, when the user want to use the non programmatic way of emitting schema they need that dependency. As well, when they want to use the programmatic way, they need to referthat dependency directly.
Describe alternatives you've considered It's not easy to isolate some parts of a library. Maybe tree shaking can help but it's not easy.
Have you tried bundling the app and configuring webpack/other bundler to use dummy mock instead of node.js packages?
Something like this for browser: https://github.com/MichalLytek/type-graphql/blob/master/examples/apollo-client/package.json#L19-L23
I try to see if this is possible with vercel@edge where you actually have no control of the bundler
it looks like there's no similar way or, at the least, I wasn't able to find one
I mean you should bundle your project by yourself (in order to mock node.js api) and then publish the bundle to vercel edge
I see. Definitely harder but doable. Thanks
I just finished building a Next.js project with with type-graphql 2.0.0-beta.2 and love it so much. Unfortunately when trying to run my graphql route in export const runtime = 'edge';
mode in preparation to deploy to cloudflare I am getting Module not found: Can't resolve 'path'
@MichalLytek Would making emitSchemaFile
code dynamically import/require fs/path libraries only when needed fix the situation?
const path = await import("path");
before use rather than import path from "path";
I suspect if emitSchemaFile
is false then the code path would hopefully not be triggered on edge builds and this could be a way forward.
Full error
./node_modules/.pnpm/[email protected][email protected][email protected]/node_modules/type-graphql/dist/helpers/filesystem.js:5:39
Module not found: Can't resolve 'path'
https://nextjs.org/docs/messages/module-not-found
Import trace for requested module:
./node_modules/.pnpm/[email protected][email protected][email protected]/node_modules/type-graphql/dist/utils/emitSchemaDefinitionFile.js
@Enalmada as a temporary workaround I forked the project and created the package type-graphql-edge
that runs on edge environment. The option and the logic to emit schema file is removed entirely.
Another issue for the edge environment is the usage of global
that has been removed.
I tried several approach to this and trying to alias the node libraries is too hard to make it reasonable. It only works in very simple setup, but for example in mine I have a separated prisma project that uses type-grapqhl-prisma to generate code which introduce other problems. Also, type-grapqhl
relies for some reason on global
variable that is not present in the edge runtime. Since the blocking issue are simpler to solve in this library more than in the code that uses it, would you accept any PR that goes in the direction of supporting the edge runtime? This of course would involve having a separate package for writing the schema to a file. I don't know if this is in the interest of this library so let me know if it's worth to go in this direction with a pull request.
Also, this would allow using this library with prisma accelerate that instead runs on edge runtime.
Any progress on that? Would be awesome to run it with CF workers.
@akhdefault I'm only going to work on it if this is the direction the maintainer of the library wants to pursuit.
@ramiel There are no easy ways to fix that:
- making
await import
means we can't have syncbuildSchema
anymore - going with subpath or separate package approach means users lose the automatic schema emitting feature
emitSchemaFile: true
-
global
has to stay (maybe as aglobalThis
) as it's the way to prevent issues with multiple metadata storages, when you have your type definitions separated from the runtime/server part, as there's a risk of multiple version/instances oftype-graphql
in node_modules
In ideal world we would have @typegraphql/edge
package and @typegraphql/node
, but we're not ready to rewrite whole repo to support monorepo and multiple packages approach with proper build mechanism.
I think you should try to use some JS bundler to pack your source code with node_modules dependencies, just like we do for the browser: https://parceljs.org/features/node-emulation/#polyfilling-%26-excluding-builtin-node-modules
This way, as the code is not actually called, replacing the imports with empty modules won't cause any runtime issues.
So, let me reply to each point
- Use
await import
: I agree, this should not be done - going with subpath: yes, this is the only drawback I see,
emitSchemaFile:
options is lost. The alternative is not that bad though:
import {buildSchema} from 'type-graphql';
import {emitSchemaFile} from 'type-graphql-utils';
const schema = buildSchema();
emitSchemaFile({schema, filePath: 'schema.graphql'});
-
global
is not really needed or a problem. Check how I changed the code in my package here
I know that building the project mocking node library looks a good solution, but in real world complex scenario is not. For example in my monorepo, with pnpm and tsup (esbuild) the resolution of packages lead to have the alias not working in deeped imported packages (where the imports are not aliased by design).
In any case, I fully understand if this is something the project doesn't want to do. In that case, feel free to close this issue, or maybe try to understand how much of your userbase is interested in this. Well, do what you feel right, of course :D
global is not really needed or a problem. Check how I changed the code in my package here
This is a workaround. After releasing that "fix" there will be new issues opened that TypeGraphQL does not see the definitions from other packages, etc. 😕
What do you mean? The definition is the only untouched part
I mean the definitions - classes decorated with TypeGraphQL's decorators. Like someones publish Relay helpers for TypeGraphQL as an npm package - in some edge cases you might end in multiple version of type-graphql
in your node_modules. And since const cachedStorage = new MetadataStorage();
is a local declaration, the metadata storages will be separated and the built schema won't be matching your expectation (missing args, types, etc.).
Oh, I see what you mean. The current solution in type-graphql is a workaround to actually be sure there's only one instance of typegraphql in node_modules. For example it's the same with the graphql
package after all, where you cannot have two different versions installed (not even with the same version number), this is why it's usually set as a peer dependency.
Anyway, for the general discussion of this issue, I'd suggest to maybe close it. I understand this is not a direction this project wants to pursuit and there's nothing bad. I will try to come up with another package that follow yours. I also have to do the same for typegraphql-prisma. Maybe you can revisit this proposal or a similar one in the future.
Thanks for all the attention!
Anyway, for the general discussion of this issue, I'd suggest to maybe close it. I understand this is not a direction this project wants to pursuit and there's nothing bad. I will try to come up with another package that follow yours. I also have to do the same for typegraphql-prisma. Maybe you can revisit this proposal or a similar one in the future.
Supporting all runtimes is the direction I want to follow with this project. However, not everything is easily doable right now.
The issue with fs will be solved as soon as we switch to the monorepo and separate packages. Then the core could be run on any platform as it would be almost zero deps (only graphql-js).
The issue with global might be not relevant in today times.
I, and other users, had issues mostly because of npm link
and other solutions where you have multiple node_modules.
However, now with monorepos, you can dedupe the type-graphql
package and have only a single instance both in the app and in the libs. That's what graphql-js
with the "another realm" error.
So this one could be fixed soon with some conditional check for existence of global
and if not, then using local variable as storage mounting point.
This message makes me very happy. I'll also be very happy of helping if there's any need