Unused @ts-expect-error comment

[DonnyVerduijn]

Description

Given the following tsconfig.json configuration, typescript warns about an unused @ts-expect-error comment in core/request.ts at line 28 (i assume this file is input agnostic to that point but i'll share the contents.)

  "target": "ES2020",
  "useDefineForClassFields": true,
  "lib": ["ES2020", "DOM", "DOM.Iterable"],
  "module": "ESNext",
  "skipLibCheck": true,

  /* Bundler mode */
  "moduleResolution": "bundler",
  "allowImportingTsExtensions": true,
  "allowSyntheticDefaultImports": true,
  "exactOptionalPropertyTypes": true,
  "esModuleInterop": true,
  "resolveJsonModule": true,
  "isolatedModules": true,
  "noEmit": true,
  "jsx": "react-jsx",
  "jsxImportSource": "@emotion/react",

  /* Linting */
  "strict": true,

The contents of core/request.ts:

export const base64 = (str: string): string => {
  try {
    return btoa(str);
  } catch (err) {
    // @ts-expect-error
    return Buffer.from(str).toString('base64');
  }
};

OpenAPI specification (optional)

No response

Configuration

No response

System information (optional)

No response

[mrlubos]

@DonnyVerduijn does this prevent you from generating a client?

[DonnyVerduijn]

No. It's just an error in the editor. This doesn't fail tsc afaik.

[DonnyVerduijn]

It does! Didn't know that.

src/__generated/api/core/request.ts:28:5 - error TS2578: Unused '@ts-expect-error' directive.

28     // @ts-expect-error
       ~~~~~~~~~~~~~~~~~~~

src/__generated/api/services.gen.ts:104:31 - error TS2379: Argument of type '{ method: "POST"; url: string; formData: { fileName?: Blob | File; } | undefined; mediaType: string; errors: { 500: string; }; }' is not assignable to parameter of type 'ApiRequestOptions' with 'exactOptionalPropertyTypes: true'. Consider adding 'undefined' to the types of the target's properties.
  Types of property 'formData' are incompatible.
    Type '{ fileName?: Blob | File; } | undefined' is not assignable to type 'Record<string, unknown>'.
      Type 'undefined' is not assignable to type 'Record<string, unknown>'.

104     return __request(OpenAPI, {
                                  ~
105       method: 'POST',
    ~~~~~~~~~~~~~~~~~~~~~
... 
111       },
    ~~~~~~~~
112     });
    ~~~~~

Found 2 errors in 2 files.

Errors  Files
     1  src/__generated/api/core/request.ts:28
     1  src/__generated/api/services.gen.ts:104

[DonnyVerduijn]

The problem with this, is that i'm generating the client postinstall (the client is not versioned), so during a CI run, tsc would break the pipeline.

[mrlubos]

Same question as the other issue, does using a standalone client solve this?

[DonnyVerduijn]

Same question as the other issue, does using a standalone client solve this?

Yes, although i prefer using the services. It has a lot of benefits in terms of intellisense, which is the primary reason to use this generator. What are you're thoughts on that, given the effort you're putting into this?

[DonnyVerduijn]

By the way, i just had to revert @hey-api/openapi-ts back to 0.45.1 after upgrading. It seems there is a regression in 0.46.1

[mrlubos]

What's the regression @DonnyVerduijn? And to be clear, you are more than welcome to continue using services. The client itself doesn't have a good type support anyway since I expect you to use services. Have a look at the demo https://heyapi.vercel.app/openapi-ts/clients.html#fetch-api

[DonnyVerduijn]

The referenced services were missing. I cleaned the working directory to be sure and reverted all changes (even removing fetch-client from node_modules), but 0.46.1 was not able to generate the services.

[mrlubos]

What does your config look like?

[DonnyVerduijn]

import * as fs from 'fs';
import { createClient } from '@hey-api/openapi-ts';
import { load } from 'js-yaml';

export const generate = async (
  yamlFilePath: string = './openapi.yml',
  outputDir: string = './src/__generated/api'
) => {
  try {
    // Read YAML file and parse it
    const yamlContent = fs.readFileSync(yamlFilePath, 'utf8');
    const jsonSpec = load(yamlContent) as Record<string, unknown>;

    // Generate TypeScript client
    await createClient({
      client: 'fetch',
      // debug: true,
      input: Object.assign(jsonSpec, {
        servers: [{ url: '/api' }],
      }),
      output: {
        format: 'prettier',
        lint: 'eslint',
        path: outputDir,
      },
      schemas: {
        export: true,
        type: 'json',
      },
      services: {
        export: true,
        name: '{{name}}Service',
      },
      types: {
        dates: true,
        enums: 'typescript',
        export: true,
        name: 'preserve',
      },
    });

    console.log('Client generation completed successfully.');
  } catch (error) {
    console.error('Failed to generate client:', error);
  }
};

[DonnyVerduijn]

What's the regression @DonnyVerduijn? And to be clear, you are more than welcome to continue using services. The client itself doesn't have a good type support anyway since I expect you to use services. Have a look at the demo https://heyapi.vercel.app/openapi-ts/clients.html#fetch-api

There might actually be a bit of a misunderstanding in terms of what we call services. In openapi-ts 0.45.1, we have a set of services where each service is a class, that exposes its methods to call certain api endpoints. When i said it makes more sense to take this approach in terms of a better DX through intellisense, what i mean't to say is that with large apis, it becomes very hard to get an intuition when there is no categorization. So, when there is just one file that exposes the methods directly as exported functions (such as in the fetch-client example), it would really become a burden to use.

[mrlubos]

Did you follow the migration guide for 0.46.0? Sounds like you want to keep generating classes but I don't see that in your config

[DonnyVerduijn]

Did you follow the migration guide for 0.46.0? Sounds like you want to keep generating classes but I don't see that in your config

I see. I'll take a look at it tomorrow. You should really consider including that information in your releases and use semantic versioning/opt-in migration strategies.

[mrlubos]

Good shout. Do you know a good strategy for including migration notes in releases? I didn't look into configuring changesets further (there were no changelogs before btw).

The project follows semver too, see https://github.com/semver/semver/issues/333

[DonnyVerduijn]

Good shout. Do you know a good strategy for including migration notes in releases? I didn't look into configuring changesets further (there were no changelogs before btw).

The project follows semver too, see https://github.com/semver/semver/issues/333

I'm not very experienced with authoring libraries on github, but i assume it depends on how the releases are prepared. I would have to look into it to say something specific.

In terms of following semver, the point is to make it easy to track down breaking changes through the release notes and allow pinning major versions if necessary.

Most of the time, breaking changes/migrations can be delayed without hindering adoption of new features. By introducing compatibility layers that are scrapped in the next major bump, the migrations are provided OOTB during minor bumps.

[DonnyVerduijn]

This problem is resolved by using @hey-api/client-fetch. There are some other problems with @hey-api/client-fetch but closing this.