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

chore: upgrade to latest version of graphql-subscriptions #330

Merged
merged 5 commits into from
Dec 12, 2024

Conversation

kasir-barati
Copy link
Contributor

Closes #327

@kasir-barati
Copy link
Contributor Author

Ready for code review.

Copy link

nx-cloud bot commented Dec 12, 2024

☁️ Nx Cloud Report

CI is running/has finished running commands for commit b662cc1. As they complete they will appear below. Click to see the status, the terminal output, and the build insights.

📂 See all runs for this CI Pipeline Execution


🟥 Failed Commands
nx run-many --target=lint --all
✅ Successfully ran 4 targets

Sent with 💌 from NxCloud.

@kasir-barati
Copy link
Contributor Author

Off-topic: BTW I was wondering if we could also upgrade our legacy tsconfig to a more up to date one. Right now we have "moduleResolution": "node", to "NodeNext" since if I am not wrong is a new resolution mode designed for NodeJS's specific implementation of co-existing ESM and CommonJS.

@kasir-barati
Copy link
Contributor Author

Checking the failing pipelines

@kasir-barati
Copy link
Contributor Author

I am lost, it is kinda bugging me why query-sequelize linter broke down. But I think it is related to TS:

microsoft/TypeScript#35186
And after I followed the instructions given here: microsoft/TypeScript#33133

I ended up with this log:

 NX   Debug Failure. No error for last overload signature

Call:this.sqlComparisonBuilder.build(colName, cmpType, cmp[cmpType] as EntityComparisonField<Entity, T>, alias)
Declarations:   build<F extends keyof Entity>(
    field: F,
    cmp: FilterComparisonOperators<Entity[F]>,
    val: EntityComparisonField<Entity, F>,
    alias?: string
  ): WhereOptions {
    const col = alias ? `$${alias}.${field as string}$` : `${field as string}`
    const normalizedCmp = (cmp as string).toLowerCase()
    if (this.comparisonMap[normalizedCmp]) {
      // comparison operator (e.b. =, !=, >, <)
      return { [col]: { [this.comparisonMap[normalizedCmp]]: val } }
    }
    if (normalizedCmp === 'between') {
      // between comparison (field BETWEEN x AND y)
      return this.betweenComparisonSQL(col, val)
    }
    if (normalizedCmp === 'notbetween') {
      // notBetween comparison (field NOT BETWEEN x AND y)
      return this.notBetweenComparisonSQL(col, val)
    }
    throw new Error(`unknown operator ${JSON.stringify(cmp)}`)
  }
Occurred while linting /home/kasir/projects/nestjs-query/packages/query-sequelize/src/query/where.builder.ts:85
Rule: "@typescript-eslint/no-unsafe-return"

Error: Debug Failure. No error for last overload signature
Call:this.sqlComparisonBuilder.build(colName, cmpType, cmp[cmpType] as EntityComparisonField<Entity, T>, alias)
Declarations:   build<F extends keyof Entity>(
    field: F,
    cmp: FilterComparisonOperators<Entity[F]>,
    val: EntityComparisonField<Entity, F>,
    alias?: string
  ): WhereOptions {
    const col = alias ? `$${alias}.${field as string}$` : `${field as string}`
    const normalizedCmp = (cmp as string).toLowerCase()
    if (this.comparisonMap[normalizedCmp]) {
      // comparison operator (e.b. =, !=, >, <)
      return { [col]: { [this.comparisonMap[normalizedCmp]]: val } }
    }
    if (normalizedCmp === 'between') {
      // between comparison (field BETWEEN x AND y)
      return this.betweenComparisonSQL(col, val)
    }
    if (normalizedCmp === 'notbetween') {
      // notBetween comparison (field NOT BETWEEN x AND y)
      return this.notBetweenComparisonSQL(col, val)
    }
    throw new Error(`unknown operator ${JSON.stringify(cmp)}`)
  }
Occurred while linting /home/kasir/projects/nestjs-query/packages/query-sequelize/src/query/where.builder.ts:85
Rule: "@typescript-eslint/no-unsafe-return"
    at resolveCall (/home/kasir/projects/nestjs-query/node_modules/typescript/lib/typescript.js:79290:35)
    at resolveCallExpression (/home/kasir/projects/nestjs-query/node_modules/typescript/lib/typescript.js:79684:12)
    at resolveSignature (/home/kasir/projects/nestjs-query/node_modules/typescript/lib/typescript.js:80077:16)
    at getResolvedSignature (/home/kasir/projects/nestjs-query/node_modules/typescript/lib/typescript.js:80103:18)
    at checkCallExpression (/home/kasir/projects/nestjs-query/node_modules/typescript/lib/typescript.js:80214:23)
    at checkExpressionWorker (/home/kasir/projects/nestjs-query/node_modules/typescript/lib/typescript.js:83473:16)
    at checkExpression (/home/kasir/projects/nestjs-query/node_modules/typescript/lib/typescript.js:83383:32)
    at getTypeOfExpression (/home/kasir/projects/nestjs-query/node_modules/typescript/lib/typescript.js:83324:18)
    at getRegularTypeOfExpression (/home/kasir/projects/nestjs-query/node_modules/typescript/lib/typescript.js:89791:40)
    at getTypeOfNode (/home/kasir/projects/nestjs-query/node_modules/typescript/lib/typescript.js:89703:14)

———————————————————————————————————————————————————————————————————————————————————————————————————————————————————————————————————————————————————————————————

 NX   Running target lint for 7 projects failed

What I can do to fix this is not really clear to me. Any comment?

@TriPSs
Copy link
Owner

TriPSs commented Dec 12, 2024

I'm having the same, you can ignore it.

@kasir-barati
Copy link
Contributor Author

BTW I also like to bump the TS version in this PR if you're OK with it since it is just a patch version update (from 5.5.4 to 5.7.2).

@TriPSs
Copy link
Owner

TriPSs commented Dec 12, 2024

I just merged my PR that also updates a couple of deps so you are welcome to update more :)

@kasir-barati
Copy link
Contributor Author

kasir-barati commented Dec 12, 2024

BTW I guess we might wanna introduce a major version for this. I mean this can introduce breaking changes for those who are still using older versions of graphql-subscriptions or its implementations. Any comment on this @TriPSs?

And if you agree please lemme know if I need to do it or you'll need to do it personally. Thanks in advance.

@TriPSs
Copy link
Owner

TriPSs commented Dec 12, 2024

Yea good one, if you can add a commit that looks like:

feat(query-graphql): Updated graphql subscriptions

BREAKING CHANGE: Updated to new major version of graphql-subscriptions

BREAKING CHANGE: Updated to new major version of graphql-subscriptions
@TriPSs TriPSs merged commit 6f9c1a2 into TriPSs:master Dec 12, 2024
12 of 14 checks passed
@TriPSs
Copy link
Owner

TriPSs commented Dec 12, 2024

Thanks for the PR!

TriPSs added a commit that referenced this pull request Dec 13, 2024
Sorry for doing it haphazardly 🙇.

So here I increased the missing peerDep version.

Related to: #330
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.

unmet peer graphql-subscriptions@^2.0.0: found 3.0.0
2 participants