Skip to content

Decorators: @mutation, @query and @subscription - #16

Merged
AngelEVargas merged 3 commits into
feature/graphqlfrom
feature/avs-opkind-decorators
Mar 19, 2025
Merged

Decorators: @mutation, @query and @subscription#16
AngelEVargas merged 3 commits into
feature/graphqlfrom
feature/avs-opkind-decorators

Conversation

@AngelEVargas

Copy link
Copy Markdown

Adding decorators to indicate the kind of GraphQL operation.

@query
op read(): string;

@mutation
op update(): string;

@subscription
op get_periodically(): string

GraphQLOperationKind
>(GraphQLKeys.operationKind);

function validateOperationKindUniqueOnNode(context: DecoratorContext, operation: Operation) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

You could also use the builtin validateDecoratorNotOnType, e.g. have the @query decorator validate that the @mutation decorator and the @subscription decorator are not also applied.

I'm trying to think why that might be preferable. Perhaps there could be a scenario where the state is set but the other decorators aren't applied and we want to allow that.
I haven't tried this, but maybe something like

interface Get<T extends {}> {
  @get foo(): T[] | Error;
}

interface Widgets extends Get<Widget> {
  @post foo(...Widget): void
}

interface WidgetsAgain extends Get<Widget> {
  @put foo(...Widget): Widget;
}

would exercise a case where there is a difference between checking the state and checking the decorators. For instance, the HTTP verb decorators look for other decorators, even though the state handling is very similar:

const verbDecorators = type.decorators.filter(
(x) =>
VERB_DECORATORS.includes(x.decorator) &&
x.node?.kind === SyntaxKind.DecoratorExpression &&
x.node?.parent === type.node,
);

What I would do is look in the existing code for similar behavior.

Comment thread packages/graphql/src/lib.ts Outdated
Comment on lines +122 to +126
export const {
reportDiagnostic,
createDiagnostic,
stateKeys: GraphQLKeys,
} = $lib;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Are you using eslint/prettier as defined in the TypeSpec repo? We should be sure to follow the same style rules as upstream.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Can we make this change first in a (very small) separate commit?

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Do we need these types? We don't seem to be using them anywhere, and I don't see other emitters declaring types for all their decorators.

Indeed, from a type level, type MutationDecorator, type QueryDecorator, and type SubscriptionDecorator are all identical.

@AngelEVargas
AngelEVargas merged commit 43bcaa3 into feature/graphql Mar 19, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants