Decorators break completion

[sdykae]

Actual Behavior

Expected Behavior

Steps to Reproduce the Problem Or Description

Specifications

• GraphQL for VSCode Extension Version: 0.3.18

• VSCode Version: latest

• OS Name: linux

• OS Version: ubuntu 20.04

#fix
https://github.com/babel/babel-eslint/issues/774 https://github.com/babel/babel-eslint/issues/662 https://stackoverflow.com/questions/53483066/babel-7-using-the-export-keyword-between-a-decorator-and-a-class-is-not-allowed/54516632

Why babel-eslint is triggered? Wtf? My project don't use babel, or eslint.

[acao]

we use babel in the language server for parsing javascript and typescript, and finding tags

there is a known bug where decorators break the language server

[sdykae]

Is there a way to pass those options

parserOptions: { ecmaVersion: 2015, ecmaFeatures: { legacyDecorators: true } }

to fix it? @acao

[tfp]

This is bugging me for months now. It's so annoying to comment out/in the decorators just so the autocomplete works.

[JonatanE]

Bugging me as well. Breaks autocomplete for all my angular services

[fajarclodeo]

Affect my nestjs project as well.

[acivier-serial]

Any update on this issue ?

[acao]

I’m in the midst of a big project at work, but if someone wants to open a PR with those changes to the babel parser options and a test for findGraphQLTags that proves the fix, it would be very welcome!

[acao]

At this service level (@babel/parser) we have decorators-legacy available, however I get this error whenever we execute parse() on any file in the language server:

we can add an option to allow someone to optionally enable support for decorators-legacy, which would disable support for decorators? one could use projects to filter which files have this setting enabled?

[joepjoosten]

https://github.com/graphql/graphiql/blob/48e7710a80739931565e1d472ed38136cc73865f/packages/graphql-language-service-server/src/findGraphQLTags.ts#L43

[jrwpatterson]

any updates on this. is there a way just to block it looking at the typescript with decorators, we are currently running a monorepo with the graph queries defined in a react app and a nest server. The nest server doesn't need any graph queries but because it has decorators its breaking the rest of the thing..

[acao]

If someone wants to open a PR it’s a relatively simple option to add opt in for legacy decorators. We can’t just enable support for legacy decorators, because then it would break the working support for decorators

[mvnrhd]

@acao I've added the changes in this fork: https://github.com/mvnrhd/graphiql/tree/feat/add-opt-in-for-legacy-decorators. Unfortunately, I didn't manage to fully test my changes locally and tests are missing as well. But maybe you (or someone else) could use it as a basis for addressing the issue soon? 🚀

[acao]

@mvnrhd indeed, happy to do that! Thank you!

[mvnrhd]

Hey @acao, do you think you could tackle this soon? 🙂

[acao]

@mvnrhd hey this is awesome, thank you! finally had a chance to take a look

The only thing is that it's a vscode setting only, with no graphql-config options yet -

This means non-vscode users might have more trouble opting in to this feature - for example most nvim coc users just configure the graphql config for the LSP server, and aren't used to needing to set extension settings.

It also means you can't have it enabled for a single project, and disabled for others - it's all or nothing.

We just need to support this config coming from the matching graphql-config project or vscode settings. Do you think you can add that to the branch? I see this bug has 9 watchers which means it's one of our top priority bugs with the LSP - this is the one that blocks nest.js and others yes?

[acao]

so here, we need to retrieve the project from the URI, and decide whether to use legacy decorators or not for the project for that URI

For example, we use this approach to allow you to configure whether you want validation enabled on a per-project basis

https://github.com/graphql/graphiql/compare/main...mvnrhd:graphiql:feat/add-opt-in-for-legacy-decorators#diff-7757a3768d45bacff6ba6fdef1dd9aff1b7174c99f2bdac30374d0129ad69f09L377

[mvnrhd]

Thanks for the implementation hints @acao. Since I'm quite busy atm not sure if I can make it this week. Sounds straightforward though.

[mvnrhd]

@acao I've incorporated your suggestions in commit https://github.com/graphql/graphiql/commit/02923a5cb39f4eb59b3c955e350ad34992a3e410. Hope that covers the use case you've mentioned. 🙏

[acao]

Thanks for this @mvnrhd! Almost feels excessive in retrospect. I've been thinking about this fix for a long time, making it happen tonight! Sorry it's been so long everyone

[acao]

After finally writing some unit tests, from what I can tell, enabling decorators-legacy actually allows both legacy es7 decorators syntax and the current syntax. I will leave decorators-legacy enabled instead of decorators because it's less restrictive, and decorators offers nothing new.

[acao]

the latest [email protected] and related lsp server version should fix this! thanks everyone