fixed missing import from graphql-ws.ts

[nizamani]

file graphql-ws.ts had missing import WebSocket which is being imported from ws dependency. This causes project to not build/compile in nestjs

[jasonkuhrt]

Can you share what the error was? WebSocket is a global in the DOM. It sounds like your backend code pulled in some frontend code? A broken test would help.

[EmirBoyaci]

Can you share what the error was? WebSocket is a global in the DOM. It sounds like your backend code pulled in some frontend code? A broken test would help.

This PR is related to #385 and you can see error from there: #330 (comment)

[EmirBoyaci]

Can you share what the error was? WebSocket is a global in the DOM. It sounds like your backend code pulled in some frontend code? A broken test would help.

It is because we are not in the DOM, we are in the backend (iow node environment) so while building via tsc it throws error.

[nizamani]

Can you share what the error was? WebSocket is a global in the DOM. It sounds like your backend code pulled in some frontend code? A broken test would help.

It is because we are not in the DOM, we are in the backend (iow node environment) so while building via tsc it throws error.

Yes exactly right, code doesn't compiles with tsc. A broken test is not possible when compilation itself fails

[EmirBoyaci]

Can you share what the error was? WebSocket is a global in the DOM. It sounds like your backend code pulled in some frontend code? A broken test would help.

It is because we are not in the DOM, we are in the backend (iow node environment) so while building via tsc it throws error.

Yes exactly right, code doesn't compiles with tsc. A broken test is not possible when compilation itself fails

If needed, I think we can add test by adding mock tsconfig with node environment and simply running npx tsc. I do not think it is necessary because it will never came again if we merge this PR.

[jasonkuhrt]

These kinds of project configuration issues are easy to regress on. A solution would be to have a test that scaffolds a project, installs the package, via yalc perhaps, and tries to do a build of the test project. This is a heavy test but the reality of covering stuff like this. I'll get around to writing a test for this myself but one is welcome. I've written a lot of tests like this before so have a pretty opinionated way I'll go about it, using konn. If nothing else, what would help is if you provide an absolute minimal project reproduction. Could be something I can clone or just a comment with the files and their contents needed for the repro. Thanks!

[EmirBoyaci]

These kinds of project configuration issues are easy to regress on. A solution would be to have a test that scaffolds a project, installs the package, via yalc perhaps, and tries to do a build of the test project. This is a heavy test but the reality of covering stuff like this. I'll get around to writing a test for this myself but one is welcome. I've written a lot of tests like this before so have a pretty opinionated way I'll go about it, using konn. If nothing else, what would help is if you provide an absolute minimal project reproduction. Could be something I can clone or just a comment with the files and their contents needed for the repro. Thanks!

You can check error from reproducer repository: https://github.com/EmirBoyaci/graphql-request-tsc-error

You just need to clone, run npm ci and run npm run build. Then you will see:

[jasonkuhrt]

@EmirBoyaci because the test might take some time to actually implement, there might be some value breaking apart the fix from the test. Can you just drop a TODO about it and then get the tests passing? That should be enough for this PR!

As for right now you can skip lib check right?

[EmirBoyaci]

@EmirBoyaci because the test might take some time to actually implement, there might be some value breaking apart the fix from the test. Can you just drop a TODO about it and then get the tests passing? That should be enough for this PR!

Owner of this PR is @nizamani , I think he should make that :)

Yes, as a workaround I can do that but we are using shared tsconfig in our company, so fixing this issue would be really great in short tearm (if possible :D)

[jasonkuhrt]

Yeah I understand a library being the cause of skip lib check is quite bad!

[nizamani]

@jasonkuhrt those changes are made,

• parseMessage input data was changed to any, reason being data type is not per-determined. data can be one of following, (string | Buffer | ArrayBuffer | Buffer[])
• added TODO for regression test in tsconfig.json with reference to PR link

[jasonkuhrt]

parseMessage input data was changed to any, reason being data type is not per-determined. data can be one of following, (string | Buffer | ArrayBuffer | Buffer[])

Then can we make the type reflect that? We shouldn't have to resort to any?

[nizamani]

I tried making data input of parseMessage to have type string | Buffer | ArrayBuffer | Buffer[]. Now this causes another issue causing parse function called inside parseMessage to complain that parse needs data to be of type string. And parse function actually uses data like this: JSON.parse(data)
I don't have that much deeper understanding of project to know how all this is connected. While I agree that any is not best solution. But I don't think parse input data should have type string | Buffer | ArrayBuffer | Buffer[]
What are your thoughts on this?

[ckifer]

Any movement on this PR? this issue still happens

[jonkoops]

@nizamani are you still interested in landing this? Could you rebase your PR and get it back into a mergable state?

[jonkoops]

Closing this due to a lack of response from the author of the PR. If anyone is interested in landing this please open up a new PR.