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

Towards TypeGraphQL v2.0 #1418

Open
MichalLytek opened this issue Feb 8, 2023 · 62 comments
Open

Towards TypeGraphQL v2.0 #1418

MichalLytek opened this issue Feb 8, 2023 · 62 comments
Assignees
Labels
Discussion 💬 Brainstorm about the idea Help Wanted 🆘 Extra attention is needed

Comments

@MichalLytek
Copy link
Owner

TypeGraphQL v2.0

The 2.0 release won't be a rewrite from scratch nor a breaking change in the existing API 👀

Basically, two main goals of this release are:

  1. GraphQL v16 upgrade - this is a peer dependency, so it forces bumping version to a new major release ⚠️
  2. fixes, features and cleanups that might break compatibility in some edge cases

The 1) is mostly done (#1331) however it might need some attention around examples that might not work properly or still need updating deps (e.g. Apollo Server v4).

About the rest, all is described in the changelog:
https://github.com/MichalLytek/type-graphql/blob/master/CHANGELOG.md#unreleased

We're also working with @carlocorradini on reviving the repo and tools configuration that went out-of-date since the project was born 5 years ago, like TSLint has died and Gulp is not used anymore. See the work on #1400.
This should allow external contributors to work on the codebase, implement other fixes and features.

The current state of the art is published as a beta release - npm i type-graphql@next:
https://www.npmjs.com/package/type-graphql/v/2.0.0-beta.1

Stay tuned for more info! 📻

PS No ETA yet, the beta releases are stable enough to be used by users.

@MichalLytek MichalLytek added Help Wanted 🆘 Extra attention is needed Discussion 💬 Brainstorm about the idea labels Feb 8, 2023
@MichalLytek MichalLytek self-assigned this Feb 8, 2023
@MichalLytek MichalLytek pinned this issue Feb 8, 2023
@MichalLytek
Copy link
Owner Author

MichalLytek commented Feb 9, 2023

Launch control checklist

  • automatic release with GitHub Actions when a new tag is pushed to main branch @carlocorradini
  • cancel in-progress CI checks when fresh commits were pushed @carlocorradini
  • remove resolvers glob path (resolvers: [__dirname + "/migration/*.ts"],) @MichalLytek
  • remove isAbstract legacy decorator option @MichalLytek
  • double build support (CJS and ESModules) @carlocorradini
  • replace custom scalars with graphql-scalars library @MichalLytek
  • errors thrown at runtime should extend GraphQLError and not Error @carlocorradini
  • enable schema check for buildSchemaSync @MichalLytek
  • making class-validator optional peer dependency @MichalLytek
  • upgrade to TypeScript 5.0 with all the goodness (like const) but without new decorators @MichalLytek
  • move the examples' dependencies from root package.json to the appropriate package. json @carlocorradini
  • ...

Repository owner deleted a comment from carlocorradini Feb 10, 2023
@carlocorradini

This comment was marked as resolved.

Repository owner deleted a comment from carlocorradini Feb 14, 2023
Repository owner deleted a comment from carlocorradini Feb 14, 2023
@MichalLytek
Copy link
Owner Author

@carlocorradini Comments in the code are not API docs. We would need to write full JSDoc and then show it in the *d.ts files for intelisense purposes. See #17 👀

@carlocorradini
Copy link
Contributor

Yup, I cannot find the issue from typescript repo where they propose to retain comments in JS :/

@carlocorradini
Copy link
Contributor

Yup, I cannot find the issue from typescript repo where they propose to retain comments in JS :/

TypeScript issue: microsoft/TypeScript#14619

Repository owner deleted a comment from carlocorradini Feb 17, 2023
Repository owner deleted a comment from carlocorradini Feb 17, 2023
Repository owner deleted a comment from carlocorradini Feb 17, 2023
@carlocorradini

This comment was marked as outdated.

@carlocorradini

This comment was marked as outdated.

@carlocorradini

This comment was marked as outdated.

@AChangXD
Copy link

AChangXD commented Apr 1, 2023

Any updates to the ETA?

@carlocorradini
Copy link
Contributor

Any updates to the ETA?

Nope 😅

@AChangXD
Copy link

AChangXD commented Apr 4, 2023

Seems like the argument validation messages stopped showing which field caused it with v2.0, similar to #362. Is it a known bug?

Note: validation still works, just the error message stopped showing the field that caused it.

@carlocorradini
Copy link
Contributor

carlocorradini commented Apr 4, 2023

@AChangXD See #1397

Moreover, in #1400, all runtime errors now extends GraphQLError and have additional information in extensions field (i.e. validationErrors for ArgumentValidationError)

@john-dodson
Copy link

john-dodson commented Apr 10, 2023

Field resolvers don't work with Apollo Federation v2. It seems that if you are resolving a field for a type that was resolved by a different subgraph the field resolver function will receive an object that looks like this instead of the Root dataValues themselves:

{
  _changed: {...},
  _options: {...},
  _previousDataValues: {...},
  dataValues: {...},
}

I had to write this decorator and apply it to all Field Resolvers to resolve the issue:

export function DataValues() {
  return (_target: any, _propertyKey: string, descriptor: TypedPropertyDescriptor<(...params: any[]) => Promise<any> | any>): any => {
    const originalMethod = descriptor.value
    if (!originalMethod || typeof originalMethod !== 'function') {
      throw new Error('Method not found or is not a function')
    }

    descriptor.value = async function(...args): Promise<any> {
      const callArgs = args.map((arg) => {
        return { ...arg, ...(arg.dataValues || {}) }
      })
      const result = await originalMethod.apply(this, callArgs)
      return result
    }
  }
}

@shellscape
Copy link

shellscape commented Apr 25, 2023

@MichalLytek not being able to use graphql@16 is starting to become a major blocker. All of the tools based on graphql are starting to leverage it and we're finding that many of the updates contain critical features we need to use. graphql@16 will be 1.5 years old very soon, and @17 is already in testing.

What can be done to get a release out that supports graphql@16 given that the update to v2.0 is taking quite a while?

Could we get a 2.0.0-beta.2 version published off of the latest changes in master?

@shellscape
Copy link

Moreover, in #1400, all runtime errors now extends GraphQLError and have additional information in extensions field (i.e. validationErrors for ArgumentValidationError)

@carlocorradini I don't see any changes in #1400 around GraphQLError. I would also recommend following the guild's lead on that as outlined here: https://the-guild.dev/graphql/yoga-server/docs/features/error-masking. Masking the error is going to be preferable to always throwing GraphQLError as most production systems don't want to bleed internal info to end users. We're using graphql-yoga to serve the schema that typegraphql-prisma generates, and throwing GraphQLError will instruct graphql-yoga to allow those errors to be delivered without masking to the end user.

@carlocorradini
Copy link
Contributor

@shellscape
All these errors extends GraphQLError:

  1. ArgumentValidationError

    export class ArgumentValidationError extends GraphQLError {
      public constructor(validationErrors: ValidationError[]) {
        super("Argument Validation Error", {
          extensions: {
            code: "BAD_USER_INPUT",
            validationErrors,
          }
        });
      }
    }
  2. AuthenticationError

    export class AuthenticationError extends GraphQLError {
      public constructor(
        message = "Access denied! You need to be authenticated to perform this action!",
      ) {
        super(message, { extensions: { code: "UNAUTHENTICATED" } });
      }
    }
  3. AuthorizationError

    export class AuthorizationError extends GraphQLError {
      public constructor(
        message = "Access denied! You don't have permission for this action!")
      {
        super(message, { extensions: { code: "UNAUTHORIZED" } });
      }
    }

All of the above errors do not disclose any sensible information, hence they shouldn't be hidden. If you really want to hide them, just catch the error and return a more generic one.

@shellscape
Copy link

All these errors extends GraphQLError

great, those should be fine.

If you really want to hide them, just catch the error and return a more generic one.

That's reasonable using this lib directly, not so much when using with the sister lib typegraphql-prisma.

@carlocorradini
Copy link
Contributor

@shellscape
This is straightforward.
See https://www.apollographql.com/docs/apollo-server/data/errors as an example.
There is no difference between typegraphql and typegraphql-prisma since the error handling is done in/by the server and not in the "logic".

@vany0114
Copy link

vany0114 commented Aug 8, 2023

Hi @MichalLytek I'm curious if is there a way to inject the following in the schema or type-graphql already supports the federation v2 out of the box?

extend schema
  @link(url: "https://specs.apollo.dev/federation/v2.3",
        import: ["@key", "@shareable"])

@carlocorradini
Copy link
Contributor

carlocorradini commented Aug 8, 2023

@vany0114 See this

It's currently in my repo but it will be merged soon (see PR #1400)

@vany0114
Copy link

vany0114 commented Aug 8, 2023

@carlocorradini Thanks! do you guys have a sense of when it's gonna be released?

@carlocorradini
Copy link
Contributor

@vany0114 This week it should be merged.
I don't know the exact date for the release, let's wait @MichalLytek
:)

@vany0114
Copy link

vany0114 commented Aug 8, 2023

@carlocorradini gotcha, in the meantime can you point me to where is that implemented to see if I can borrow it while it is released? 🙏

@carlocorradini
Copy link
Contributor

carlocorradini commented Aug 8, 2023

Simply run:

npm uninstall type-graphql
npm install type-graphql@next

Easy peasy 😎

Copy paste the federation V2 example and you are good to go 🥳

@vany0114
Copy link

vany0114 commented Aug 8, 2023

I guess that would install the "unofficial" package and I'd like to avoid it, so if the code that extends the schema is not too complex I'd like to incorporate it on my own buildFederatedSchema while your PR is released so that the schema contains this: and can be considered by Apollo that is using the federation v2.

extend schema
  @link(url: "https://specs.apollo.dev/federation/v2.3",
        import: ["@key", "@shareable"])

@vany0114
Copy link

vany0114 commented Aug 9, 2023

@MichalLytek not sure if I misunderstood but in this comment you say that the extend schema stuff is added automatically by the buildSubgraphSchema

It's added automatically by buildSubgraphSchema - see helpers/buildFederatedSchema

However, I was reviewing the implementation and it seems it's not, also the documentation says it needs to be added in the schema.

So I'm curious whether the opt-in to Federation 2 is really supported (or it's gonna be) by type-graphql 🤔

@carlocorradini
Copy link
Contributor

PR #1400 has been merged! 🥳😎🤩

@RealSebbooo
Copy link

Hello everyone, i am struggling to use the buildSchema function. Can someone tell me how to convert my glob path strings into resolver classes to use them in the buildSchema function.

@carlocorradini
Copy link
Contributor

@RealSebbooo
Glob paths resolvers option has been removed. See CHANGELOG.md

@RealSebbooo
Copy link

And what is the alternative so that i can import my resolvers from a different file?

@MichalLytek
Copy link
Owner Author

@RealSebbooo You should just use imports and resolvers array. Dummy require might produce weird issues and is not supported anymore.

Repository owner deleted a comment from RealSebbooo Aug 9, 2023
@john-landgrave
Copy link

Just checking in because it's been a few months. How are things going on v2?

@MichalLytek
Copy link
Owner Author

@john-landgrave There is a pending refactor/rewrite of the subscriptions support as the old package is not maintained anymore:
#1578

Then I need to make sure all issues requiring breaking changes are implemented, write some migrations docs and we should be ready to launch!

@MichalLytek
Copy link
Owner Author

MichalLytek commented Jan 3, 2024

The 4th beta release has been published! 🚀
https://www.npmjs.com/package/type-graphql/v/2.0.0-beta.4

As you can read in the changelog, it has new features for directives, validation and mostly subscriptions - now it uses the @graphql-yoga/subscriptions package, instead of old and abandoned graphql-subscriptions.
It accepts any PubSub implementation that matches the thin interface, so you can use any other package you want, if the yoga's one does not fulfill your needs 😉

There's also a migration guide for this major and big breaking change:
https://typegraphql.com/docs/migration-guide.html#subscriptions

Feel free to leave here any feedback about the improved subscriptions support! 🙏

@john-landgrave
Copy link

@MichalLytek thanks for the update! What version of Prisma will v2 require?

@MichalLytek
Copy link
Owner Author

@john-landgrave We're on the core TypeGraphQL repo, not the Prisma integration 😉

@john-landgrave
Copy link

john-landgrave commented Jan 4, 2024 via email

@eduardolundgren
Copy link

It accepts any PubSub implementation that matches the thin interface, so you can use any other package you want, if the yoga's one does not fulfill your needs 😉

@MichalLytek, I appreciate the latest update. You mentioned that it should be compatible with any PubSub adhering to the interface. However, I've encountered an issue: when attempting to use RedisPubSub as PubSub, the Apollo client fails to connect. This makes me curious if there might be a bug within the new default Yoga implementation's internal assumptions.

Internal error occurred during message handling. Please check your implementation. TypeError: source[Symbol.asyncIterator] is not a function
    at Object.executor (/packages/api/node_modules/.pnpm/@[email protected]/node_modules/@graphql-yoga/subscription/cjs/operator/filter.js:7:54)
    at /packages/api/node_modules/.pnpm/@[email protected]/node_modules/@repeaterjs/repeater/cjs/repeater.js:453:69
    at new Promise (<anonymous>)
    at execute (/packages/api/node_modules/.pnpm/@[email protected]/node_modules/@repeaterjs/repeater/cjs/repeater.js:453:19)
    at Repeater.next (/packages/api/node_modules/.pnpm/@[email protected]/node_modules/@repeaterjs/repeater/cjs/repeater.js:484:13)
    at Object.next (/packages/api/node_modules/.pnpm/[email protected]/node_modules/graphql/execution/mapAsyncIterator.js:43:39)
    at onMessage (/packages/api/node_modules/.pnpm/[email protected][email protected]/node_modules/graphql-ws/lib/server.js:210:180)
    at processTicksAndRejections (node:internal/process/task_queues:95:5)
    at async WebSocket.<anonymous> (/packages/api/node_modules/.pnpm/[email protected][email protected]/node_modules/graphql-ws/lib/use/ws.js:83:21)

@eduardolundgren
Copy link

It accepts any PubSub implementation that matches the thin interface, so you can use any other package you want, if the yoga's one does not fulfill your needs 😉

@MichalLytek, I appreciate the latest update. You mentioned that it should be compatible with any PubSub adhering to the interface. However, I've encountered an issue: when attempting to use RedisPubSub as PubSub, the Apollo client fails to connect. This makes me curious if there might be a bug within the new default Yoga implementation's internal assumptions.

Internal error occurred during message handling. Please check your implementation. TypeError: source[Symbol.asyncIterator] is not a function
    at Object.executor (/packages/api/node_modules/.pnpm/@[email protected]/node_modules/@graphql-yoga/subscription/cjs/operator/filter.js:7:54)
    at /packages/api/node_modules/.pnpm/@[email protected]/node_modules/@repeaterjs/repeater/cjs/repeater.js:453:69
    at new Promise (<anonymous>)
    at execute (/packages/api/node_modules/.pnpm/@[email protected]/node_modules/@repeaterjs/repeater/cjs/repeater.js:453:19)
    at Repeater.next (/packages/api/node_modules/.pnpm/@[email protected]/node_modules/@repeaterjs/repeater/cjs/repeater.js:484:13)
    at Object.next (/packages/api/node_modules/.pnpm/[email protected]/node_modules/graphql/execution/mapAsyncIterator.js:43:39)
    at onMessage (/packages/api/node_modules/.pnpm/[email protected][email protected]/node_modules/graphql-ws/lib/server.js:210:180)
    at processTicksAndRejections (node:internal/process/task_queues:95:5)
    at async WebSocket.<anonymous> (/packages/api/node_modules/.pnpm/[email protected][email protected]/node_modules/graphql-ws/lib/use/ws.js:83:21)

Did some debugging and this issue does not happen when topics is an array. Before I had topics as string.

Unfortunately, it surfaced another issue, the payload is always returning 0 instead of the desired value.

@Resolver()
export class SubscriptionResolver {
  @Subscription(() => NewMutationPayload, {
    filter: ({ payload } ) => {
      console.log('payload', payload); // PRINTING 0 <--- NEW ISSUE
      ...
    },
    topics: [SubscriptionTopics.NEW_MUTATION], // <--- FIX
  })
  onCompanyRelatedMutation(@Root() newMutation: NewMutationPayload): NewMutationPayload {
    return newMutation;
  }
}

@MichalLytek
Copy link
Owner Author

with any PubSub adhering to the interface

Yes, but graphql-subscriptions are outdated and not supported out of the box.
I haven't added a snippet or adapter in order to force people to migrate from an abandoned package.

when attempting to use RedisPubSub as PubSub

There is a redis subscription example on the repo.
I recommend to read the docs about serializer too 😉
https://the-guild.dev/graphql/yoga-server/docs/features/subscriptions#custom-serializer

@eduardolundgren
Copy link

@MichalLytek thanks for the reply, and I will give another shot this week. I noticed that recent examples have transitioned from Apollo Server to Yoga, particularly because of the subscriptions, due to the graphql-subscriptions library being outdated. This shift raises a question: why has Apollo Server been phased out in the examples in favor of Yoga?

Would it be possible to reintroduce an example that combines Apollo Server with some working subscriptions? Such an example would be invaluable for those of us looking to understand how Apollo Server can still be utilized effectively, especially in contexts where its subscription capabilities are essential.

Thank you for considering this request. Your efforts in maintaining and evolving this project are greatly appreciated!

@MichalLytek
Copy link
Owner Author

MichalLytek commented Feb 7, 2024

@eduardolundgren
The changes were made in terms of internal PubSub system. It doesn't change at all the way we expose GraphQL schema via http and websockets.

So you should just follow the steps defined in the apollo-server docs:
https://www.apollographql.com/docs/apollo-server/data/subscriptions/#enabling-subscriptions

Examples uses Yoga because they are mostly a small showcase of the feature, an entry-level solution for newcomers.
It's not intended to be used as a bootstrap point for a production app.

@eduardolundgren
Copy link

@MichalLytek, you were right. I kept my existing Apollo Server setup as described in the Apollo Server docs and just switched my subscription setup to use @graphql-yoga/subscription and @graphql-yoga/redis-event-target. Everything's working smoothly now. I also updated to 2.0.0-beta.6, so not sure if that also helped — thanks for the help!

@MichalLytek
Copy link
Owner Author

MichalLytek commented Apr 24, 2024

ℹ️ Quick update regarding 2.0 release ℹ️

The goal is to group all the breaking changes, because of major version bump.
It took a while to e.g. rework subscriptions but we're now much closer to the final release! 🚀

Today I've published the first RC release. It's also marked as the latest tag on npm, so it will be the default version which developers install.

Before releasing stable 2.0, I would like to work on those issues and PRs:
#620
#1297
#1453
#1325
#1330
#1321

Also, we need to update readme, the backstory and maybe some docs/introduction, as in the last years many more GraphQL libraries came to live 😉

Stay tuned! 📻

@codingthat
Copy link

@MichalLytek Thanks for the update! I notice that all the issues and PRs you linked to are now completed or merged (except #1453 marked as invalid, and #1330 superseded by now-merged #1680). The milestone has nothing open on it either. Does that mean readme/docs updates are the last blocker?

@MichalLytek
Copy link
Owner Author

Not 100% sure but looks like so 😉

We definitely need a good migration guide, which I started writing some time ago.
Then updating docs/readme with a proper, up to date introduction.
Finally we can schedule a 2.0 stable release with some tweets, blog posts, etc. 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Discussion 💬 Brainstorm about the idea Help Wanted 🆘 Extra attention is needed
Projects
None yet
Development

No branches or pull requests

15 participants