search

lukeautry / tsoa

Build OpenAPI-compliant REST APIs using TypeScript and Node
MIT License
3.48k stars 498 forks source link

Version 6.1.0 leads to missing packages, breaks existing builds #1573

Closed ManfredLange closed 6 months ago

ManfredLange commented 7 months ago

Sorting

Expected Behavior

I can upgrade from version 6.0.1 to 6.1.0 and expect there are no breaking changes.

Current Behavior

I just tried to upgrade from version 6.0.1 to version 6.1.0. The older version is just fine with no issues running our npm scripts.

However, with version 6.1.0 we now observe that packages are missing:

These lead to compile/build errors because those packages cannot be found.

Possible Solution

Something in the direct or indirect dependencies has changed between version 6.0.1 and 6.1.0.

The problem can be solved by applying the following in package.json:

Steps to Reproduce

      1. 4.

Context (Environment)

Version of the library: 6.1.0 Version of NodeJS: 20.11.1 Version of TypeScript: 5.4.2

I'm using pnpm instead of npm but do not believe this to be the problem. Package tsoa worked just fine until an up to version 6.0.1 with pnpm across multiple TypeScript projects. There was no need to add any other package to make tsoa work.

Detailed Description

Breaking change?

Yes, this is a breaking change as we had to add the missing packages to package.json which we didn't have to list as dependencies before.

UPDATE 10 Mar 2024

A possibly better option to resolve this is described in this comment: https://github.com/lukeautry/tsoa/issues/1573#issuecomment-1986996301

github-actions[bot] commented 7 months ago

Hello there ManfredLange 👋

Thank you for opening your very first issue in this project.

We will try to get back to you as soon as we can.👀

marudor commented 7 months ago

FYI @ManfredLange the text you wanted to provide in the copyable boxes appears to be empty.

I also encountered this today though and downgraded back to 6.0.1. Another issue besides being broken even in developent (due to @hapi/boom missing as I am using Koa) is that we now need @tsoa/cli as production dependency. The generated routes now import the Template stuff from @tsoa/cli Could we revert this? I would like to keep @tsoa/cli as a devDependency

WoH commented 7 months ago

@jackey8616 Can we fix the runtime importing the services from the cli easily?

ManfredLange commented 7 months ago

@marudor You wrote:

FYI @ManfredLange the text you wanted to provide in the copyable boxes appears to be empty.

I don't understand what you are trying to say. Would you mind rephrasing, please?

ManfredLange commented 7 months ago

Further to this: It appears that the cli was a regular dependency for tsoa in version 6.0.1 already and this has not changed to 6.1.0 However, what has changed is that version 6.1.0 of the cli is missing the dependency on @hapi/boom.

Here is a snippet from the pnpm-lock.yml file when I use tsoa version 6.0.1:

  /tsoa@6.0.1:
    resolution: {integrity: sha512-Far5BFpFvMYVMZxt87O7hOFGpa6dALQ8XHhdDytry0IhIUJnRliVXLZ2t2KGcgNw0bIwsk+2XkWra18LL6cORA==}
    engines: {node: '>=18.0.0', yarn: '>=1.9.4'}
    hasBin: true
    dependencies:
      '@tsoa/cli': 6.1.0
      '@tsoa/runtime': 6.0.0
    dev: false

Here is the equivalent snippet for tsoa 6.1.0:

  /tsoa@6.1.0:
    resolution: {integrity: sha512-qFmOQEj6+EIjUesFxqwi8f5FjjpAhPRzj5StDueM23NexJgEUjc76ysghNV5IDf4wOOUCrNgI4krgwYw6trSDg==}
    engines: {node: '>=18.0.0', yarn: '>=1.9.4'}
    hasBin: true
    dependencies:
      '@tsoa/cli': 6.1.0
      '@tsoa/runtime': 6.1.0
    dev: false

Both of these were created starting with a clean slate. By clean slate I mean that I removed node_modules, local pnpm cache and also the pnpm-lock-yml file before running pnpm install. This ensured that the only information used is the packages listed in package.json.

From that snippet I read that tsoa has a dependency on @tsoa/cli which I think is not intended, and may not be what you want. Thank you, @marudor , for pointing that out!

Furthermore, once your lock file - npm, pnpm, etc. - has picked up the newer version of cli, i.e. 6.1.0, you will observe the issue with missing @hapi/boom.

To solve the problem for tsoa 6.0.1, you need to add the cli in version 6.0.1 explicitly.

To solve the problem for tsoa 6.1.0, you need to add the cli as well as @hapi/boom to the direct dependencies.

So, it appears as if the real bug twofold:

  1. tsoa has a runtime dependeny on the cli
  2. In version 6.1.0 the cli has a dependency on @hapi/boom which needs to be added as an extra step

While item 2 is "just" an extra step, the first one needs to be addressed. You don't want the cli in a production deployment. You want as little attack surface as possible. If it's not deployed in the first place, there is not way it can be attacked.

Happy to stand corrected, in case I overlooked something.

ManfredLange commented 6 months ago

On further investigating this issue, I think there is a fundamental problem with the package tsoa. It has a dependency on the cli and on the runtime. One of these should be a regular dependency (runtime) and the other a dev dependency (cli). However, when you install package tsoa as described in the "Gettting Started" instructions, both of these packages become regular dependencies. Furthermore the newer version of the cli has the missing @hapi/boom issue.

And here is how you can resolve this issue while also avoiding the bug reported at the beginning of this discussion:

Everybody who follows the "Getting Started" instructions at https://tsoa-community.github.io/docs/getting-started.html will add package tsoa to their project. As a result all of them will have the cli as a runtime dependency!

I ran this little experiment:

  1. Remove package tsoa
  2. Add package @tsoa/runtime as a regular dependency
  3. Add package @tsoa/cli as a devDependency
  4. Replace imports of tsoa with imports from @tsoa/runtime

Then clear your npm cache, delete node_modules, lock file, etc. to start with a clean slate. Restore packages, build and run your tests.

With this setup I managed to change the cli to a dev dependency. Also, this resolves the missing @hapy/boom issue for both, version 6.0.1 and 6.1.0 of tsoa (which as a single package I no longer use at the moment).

So I am wondering if this may be the best way to resolve this issue:

This has the additional advantage that everybody should be able to remove the cli from their production deployments.

Again, happy to stand corrected in case I overlooked something. So far, I have applied this solution to two repositories and so far there don't seem to be unwanted side effects.

As a point of reference, the dependencies in my pnpm-lock.json file look as follows:

   /@tsoa/cli@6.1.0:
    resolution: {integrity: sha512-fAYdR03L5bI44tz5zH0FiJUcQgnntF6fXtWfUxQf3AhuN/z4R6j2XEET8fwe0ABs7skD9a/98hqUggvKNCsIDg==}
    engines: {node: '>=18.0.0', yarn: '>=1.9.4'}
    hasBin: true
    dependencies:
      '@tsoa/runtime': 6.1.0
      '@types/multer': 1.4.11
      fs-extra: 11.2.0
      glob: 10.3.10
      handlebars: 4.7.8
      merge-anything: 5.1.7
      minimatch: 9.0.3
      ts-deepmerge: 7.0.0
      typescript: 5.4.2
      validator: 13.11.0
      yamljs: 0.3.0
      yargs: 17.7.2
    dev: true

  /@tsoa/runtime@6.1.0:
    resolution: {integrity: sha512-ZE+VMrzVV0laFjUm0j7+ox7VG0yMOiQcouCE4LdS335jMc/pA0hbcxl9EoKexTyfWgmMMFjstvQVBiAq5y60Vw==}
    engines: {node: '>=18.0.0', yarn: '>=1.9.4'}
    dependencies:
      '@types/multer': 1.4.11
      reflect-metadata: 0.2.1
      validator: 13.11.0

As desired, the cli is a dev dependency while the runtime is a regular dependency. Also, the cli has the runtime as a dependency which makes a lot of sense.

Tantalon commented 6 months ago

The generated routes.ts file imports from @tsoa/cli. These services would need to be moved from @tsoa/cli to @tsoa/runtime before @tsoa/cli could be removed from production. https://github.com/lukeautry/tsoa/blob/118f090981f90078b348987cad7a3ae65a5c8d3a/packages/cli/src/routeGeneration/templates/express/express.hbs#L5

ValentinGurkov commented 6 months ago

I just want to leave a note that this is experienced for other frameworks too, not just hapi.

jackey8616 commented 6 months ago

Messed up imports b/w cli & runtime has been take cared at #1574. Still trying to find a way out to solve hapi & koa's dependencies.

cc @WoH

ManfredLange commented 6 months ago

The generated routes.ts file imports from @tsoa/cli. These services would need to be moved from @tsoa/cli to @tsoa/runtime before @tsoa/cli could be removed from production.

https://github.com/lukeautry/tsoa/blob/118f090981f90078b348987cad7a3ae65a5c8d3a/packages/cli/src/routeGeneration/templates/express/express.hbs#L5

@Tantalon Excellent find, Robert! I didn't notice that. :+1:

jackey8616 commented 6 months ago

1574 fixes mixed import b/w @tsoa/cli & @tsoa/runtime.

1576 fixes @hapi import @ runtime, even not using such framework.

Feel free to check it out, and comment any suggestion or potential problem. apologize for the trouble cause by this refactor. thank folks!

Origin problem was bring by #1545 in case anyone want to know.

WoH commented 6 months ago

@ManfredLange closing this, will reopen if we missed something

ClementValot commented 6 months ago

Still got it with tsoa 6.1.2 as devDependency and @tsoa/runtime 6.1.1 as dependency, which are the latest releases atm :/ 

D:\.npm-global\yarn.cmd run ts-check
../../../node_modules/@tsoa/runtime/dist/routeGeneration/templates/hapi/hapiTemplateService.d.ts:1:67 - error TS2307: Cannot find module '@hapi/hapi' or its corresponding type declarations.

1 import { Request as HRequest, ResponseToolkit as HResponse } from '@hapi/hapi';
                                                                    ~~~~~~~~~~~~

../../../node_modules/@tsoa/runtime/dist/routeGeneration/templates/koa/koaTemplateService.d.ts:1:36 - error TS2307: Cannot find module 'koa' or its corresponding type declarations.

1 import type { Context, Next } from 'koa';
                                     ~~~~~

Found 2 errors in 2 files.

Errors  Files
     1  ../../../node_modules/@tsoa/runtime/dist/routeGeneration/templates/hapi/hapiTemplateService.d.ts:1
     1  ../../../node_modules/@tsoa/runtime/dist/routeGeneration/templates/koa/koaTemplateService.d.ts:1

Process finished with exit code 2
WoH commented 6 months ago

This should not be new though. If you typecheck and include node modules, including unused declarations for hapi when you're using express, then should have always had a bad time.

E: Nvm, these used to be cli declarations, where they are listed as devDep

ClementValot commented 6 months ago

I just upgraded from tsoa 6.0.1 and @tsoa/runtime 6.0.0 which didn't cause that specific issue, with the exact same setup otherwise 🤔

jackey8616 commented 6 months ago

@WoH Can we split more sub package for @hapi & koa like:

$ yarn add tsoa @tsoa/hapi

or 

$yarn add tsoa @tsoa/koa

in the future?...

jackey8616 commented 6 months ago

Will, idk, its cost too much to do such things to split .hbs and template serivce.

WoH commented 6 months ago

No, makes no sense imo. If we can import and what is already v. complicated because the package managers (pnmp) are kinda pushing this onto the user, that'll raise it as an issue here in any case. TS can still make breaking changes, so does @types/* do often enough that we can't avoid it.

WoH commented 6 months ago

I just upgraded from tsoa 6.0.1 and @tsoa/runtime 6.0.0 which didn't cause that specific issue, with the exact same setup otherwise 🤔

Upgrade to 6.1.3 please

ClementValot commented 6 months ago

I just upgraded from tsoa 6.0.1 and @tsoa/runtime 6.0.0 which didn't cause that specific issue, with the exact same setup otherwise 🤔

Upgrade to 6.1.3 please

Same issue :(

I had a quick look at the commit, shouldn't the hapi/koa dependencies be added as direct dependencies in tsoa rather than runtime? (Or something something optional dependencies but I'm not good at those) I need them when I'm in dev/build mode so that the tsoa CLI can transpile, but my tsoa-powered software won't need these at runtime