openapi-ts icon indicating copy to clipboard operation
openapi-ts copied to clipboard

Unused @ts-expect-error comment

Open donverduyn opened this issue 1 year ago • 4 comments

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

donverduyn avatar May 17 '24 16:05 donverduyn

@DonnyVerduijn does this prevent you from generating a client?

mrlubos avatar May 17 '24 17:05 mrlubos

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

donverduyn avatar May 17 '24 17:05 donverduyn

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

donverduyn avatar May 17 '24 17:05 donverduyn

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.

donverduyn avatar May 17 '24 17:05 donverduyn

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

mrlubos avatar May 24 '24 11:05 mrlubos

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?

donverduyn avatar May 24 '24 16:05 donverduyn

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

donverduyn avatar May 24 '24 17:05 donverduyn

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

mrlubos avatar May 24 '24 17:05 mrlubos

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.

donverduyn avatar May 24 '24 17:05 donverduyn

What does your config look like?

mrlubos avatar May 24 '24 18:05 mrlubos

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);
  }
};

donverduyn avatar May 24 '24 18:05 donverduyn

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.

donverduyn avatar May 24 '24 20:05 donverduyn

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

mrlubos avatar May 24 '24 22:05 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

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.

donverduyn avatar May 24 '24 23:05 donverduyn

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

mrlubos avatar May 24 '24 23:05 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

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.

donverduyn avatar May 25 '24 00:05 donverduyn

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

donverduyn avatar May 25 '24 11:05 donverduyn